diff --git a/src/dscanner/analysis/builtin_property_names.d b/src/dscanner/analysis/builtin_property_names.d index 45fe4f2..cc5a69b 100644 --- a/src/dscanner/analysis/builtin_property_names.d +++ b/src/dscanner/analysis/builtin_property_names.d @@ -5,14 +5,7 @@ module dscanner.analysis.builtin_property_names; -import std.stdio; -import std.regex; -import dparse.ast; -import dparse.lexer; import dscanner.analysis.base; -import dscanner.analysis.helpers; -import dsymbol.scope_; -import std.algorithm : map; /** * The following code should be killed with fire: @@ -27,63 +20,64 @@ import std.algorithm : map; * } * --- */ -final class BuiltinPropertyNameCheck : BaseAnalyzer -{ - alias visit = BaseAnalyzer.visit; +extern(C++) class BuiltinPropertyNameCheck(AST) : BaseAnalyzerDmd +{ + alias visit = BaseAnalyzerDmd.visit; mixin AnalyzerInfo!"builtin_property_names_check"; - this(BaseAnalyzerArguments args) + extern(D) this(string fileName) { - super(args); + super(fileName); } - override void visit(const FunctionDeclaration fd) + mixin AggregateVisit!(AST.StructDeclaration); + mixin AggregateVisit!(AST.ClassDeclaration); + mixin AggregateVisit!(AST.InterfaceDeclaration); + mixin AggregateVisit!(AST.UnionDeclaration); + + override void visit(AST.VarDeclaration vd) { - if (depth > 0 && isBuiltinProperty(fd.name.text)) - { - addErrorMessage(fd.name, KEY, generateErrorMessage(fd.name.text)); - } - fd.accept(this); + if (inAggregate && isBuiltinProperty(vd.ident.toString())) + addErrorMessage(cast(ulong) vd.loc.linnum, cast(ulong) vd.loc.charnum, + KEY, generateErrorMessage(vd.ident.toString())); } - override void visit(const FunctionBody functionBody) + override void visit(AST.FuncDeclaration fd) { - immutable int d = depth; - scope (exit) - depth = d; - depth = 0; - functionBody.accept(this); + if (inAggregate && isBuiltinProperty(fd.ident.toString())) + addErrorMessage(cast(ulong) fd.loc.linnum, cast(ulong) fd.loc.charnum, + KEY, generateErrorMessage(fd.ident.toString())); } - override void visit(const AutoDeclaration ad) + override void visit(AST.AliasDeclaration ad) { - if (depth > 0) - foreach (i; ad.parts.map!(a => a.identifier)) - { - if (isBuiltinProperty(i.text)) - addErrorMessage(i, KEY, generateErrorMessage(i.text)); - } + if (inAggregate && isBuiltinProperty(ad.ident.toString())) + addErrorMessage(cast(ulong) ad.loc.linnum, cast(ulong) ad.loc.charnum, + KEY, generateErrorMessage(ad.ident.toString())); } - override void visit(const Declarator d) + override void visit(AST.TemplateDeclaration td) { - if (depth > 0 && isBuiltinProperty(d.name.text)) - addErrorMessage(d.name, KEY, generateErrorMessage(d.name.text)); - } - - override void visit(const StructBody sb) - { - depth++; - sb.accept(this); - depth--; + if (inAggregate && isBuiltinProperty(td.ident.toString())) + addErrorMessage(cast(ulong) td.loc.linnum, cast(ulong) td.loc.charnum, + KEY, generateErrorMessage(td.ident.toString())); } private: - enum string KEY = "dscanner.confusing.builtin_property_names"; - string generateErrorMessage(string name) + template AggregateVisit(NodeType) + { + override void visit(NodeType n) + { + inAggregate++; + super.visit(n); + inAggregate--; + } + } + + extern(D) string generateErrorMessage(const(char)[] name) { import std.string : format; @@ -91,7 +85,7 @@ private: ~ " confuse code that depends on the '.%s' property of a type.", name, name); } - bool isBuiltinProperty(string name) + extern(D) bool isBuiltinProperty(const(char)[] name) { import std.algorithm : canFind; @@ -99,26 +93,25 @@ private: } enum string[] BuiltinProperties = ["init", "sizeof", "mangleof", "alignof", "stringof"]; - int depth; + int inAggregate; } unittest { import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig; + import dscanner.analysis.helpers : assertAnalyzerWarnings = assertAnalyzerWarningsDMD; + import std.stdio : stderr; StaticAnalysisConfig sac = disabledConfig(); sac.builtin_property_names_check = Check.enabled; assertAnalyzerWarnings(q{ class SomeClass { - void init(); /+ - ^^^^ [warn]: Avoid naming members 'init'. This can confuse code that depends on the '.init' property of a type. +/ - int init; /+ - ^^^^ [warn]: Avoid naming members 'init'. This can confuse code that depends on the '.init' property of a type. +/ - auto init = 10; /+ - ^^^^ [warn]: Avoid naming members 'init'. This can confuse code that depends on the '.init' property of a type. +/ + void init(); // [warn]: Avoid naming members 'init'. This can confuse code that depends on the '.init' property of a type. + int init; // [warn]: Avoid naming members 'init'. This can confuse code that depends on the '.init' property of a type. + auto init = 10; // [warn]: Avoid naming members 'init'. This can confuse code that depends on the '.init' property of a type. } }c, sac); - stderr.writeln("Unittest for NumberStyleCheck passed."); + stderr.writeln("Unittest for BuiltinPropertyNamesCheck passed."); } diff --git a/src/dscanner/analysis/run.d b/src/dscanner/analysis/run.d index a8741ba..ca7ca0b 100644 --- a/src/dscanner/analysis/run.d +++ b/src/dscanner/analysis/run.d @@ -840,10 +840,6 @@ private BaseAnalyzer[] getAnalyzersForModuleAndConfig(string fileName, checks ~= new BackwardsRangeCheck(args.setSkipTests( analysisConfig.backwards_range_check == Check.skipTests && !ut)); - if (moduleName.shouldRun!BuiltinPropertyNameCheck(analysisConfig)) - checks ~= new BuiltinPropertyNameCheck(args.setSkipTests( - analysisConfig.builtin_property_names_check == Check.skipTests && !ut)); - if (moduleName.shouldRun!CommaExpressionCheck(analysisConfig)) checks ~= new CommaExpressionCheck(args.setSkipTests( analysisConfig.comma_expression_check == Check.skipTests && !ut)); @@ -1334,6 +1330,9 @@ MessageSet analyzeDmd(string fileName, ASTBase.Module m, const char[] moduleName fileName, config.logical_precedence_check == Check.skipTests && !ut ); + + if (moduleName.shouldRunDmd!(BuiltinPropertyNameCheck!ASTBase)(config)) + visitors ~= new BuiltinPropertyNameCheck!ASTBase(fileName); foreach (visitor; visitors) {