From 6b18fa8640e25986982fa31b0c39461445600066 Mon Sep 17 00:00:00 2001 From: Hackerpilot Date: Thu, 28 May 2015 10:24:49 -0700 Subject: [PATCH] #244 --- src/analysis/config.d | 3 + src/analysis/mismatched_args.d | 224 +++++++++++++++++++++++++++------ src/analysis/run.d | 2 + 3 files changed, 189 insertions(+), 40 deletions(-) diff --git a/src/analysis/config.d b/src/analysis/config.d index 8136f05..7a81877 100644 --- a/src/analysis/config.d +++ b/src/analysis/config.d @@ -92,4 +92,7 @@ struct StaticAnalysisConfig @INI("Checks for redundant parenthesis") bool redundant_parens_check; + + @INI("Checks for mismatched argument and parameter names") + bool mismatched_args_check; } diff --git a/src/analysis/mismatched_args.d b/src/analysis/mismatched_args.d index 7848b04..bdbb14d 100644 --- a/src/analysis/mismatched_args.d +++ b/src/analysis/mismatched_args.d @@ -1,56 +1,200 @@ module analysis.mismatched_args; +import analysis.base : BaseAnalyzer; +import dsymbol.scope_; +import dsymbol.symbol; +import std.d.ast; +import std.d.lexer : tok; + +/// Checks for mismatched argument and parameter names +final class MismatchedArgumentCheck : BaseAnalyzer +{ + /// + this(string fileName, const Scope* sc) + { + super(fileName, sc); + } + + override void visit(const FunctionCallExpression fce) + { + import std.typecons : scoped; + import std.algorithm.iteration : each, map; + import std.array : array; + + if (fce.arguments is null) + return; + auto argVisitor = scoped!ArgVisitor; + argVisitor.visit(fce.arguments); + const istring[] args = argVisitor.args; + + auto identVisitor = scoped!IdentVisitor; + identVisitor.visit(fce.unaryExpression); + + DSymbol* sym = resolveSymbol(sc, identVisitor.names); + const istring[] params = sym is null ? [] : sym.parts[].map!(a => a.name).array(); + const ArgMismatch[] mismatches = compareArgsToParams(params, args); + foreach(size_t i, ref const mm; mismatches) + addErrorMessage(argVisitor.lines[i], argVisitor.columns[i], KEY, + createWarningFromMismatch(mm)); + } + + alias visit = ASTVisitor.visit; + +private: + + enum KEY = "dscanner.confusing.argument_parameter_mismatch"; +} + +final class IdentVisitor : ASTVisitor +{ + override void visit(const IdentifierOrTemplateInstance ioti) + { + import dsymbol.string_interning : internString; + + if (ioti.identifier != tok!"") + names ~= internString(ioti.identifier.text); + else + names ~= internString(ioti.templateInstance.identifier.text); + } + + override void visit(const Arguments) + { + } + + override void visit(const IndexExpression ie) + { + if (ie.unaryExpression !is null) + visit(ie.unaryExpression); + } + + alias visit = ASTVisitor.visit; + + istring[] names; +} + +final class ArgVisitor : ASTVisitor +{ + override void visit(const ArgumentList al) + { + foreach (a; al.items) + { + auto u = cast(UnaryExpression) a; + if (u !is null) + visit(u); + else + { + args ~= istring.init; + lines ~= size_t.max; + columns ~= size_t.max; + } + } + } + + override void visit(const UnaryExpression unary) + { + import dsymbol.string_interning : internString; + + if (unary.primaryExpression is null) + return; + if (unary.primaryExpression.identifierOrTemplateInstance is null) + return; + if (unary.primaryExpression.identifierOrTemplateInstance.identifier == tok!"") + return; + immutable t = unary.primaryExpression.identifierOrTemplateInstance.identifier; + lines ~= t.line; + columns ~= t.column; + args ~= internString(t.text); + } + + alias visit = ASTVisitor.visit; + + size_t[] lines; + size_t[] columns; + istring[] args; +} + +DSymbol* resolveSymbol(const Scope* sc, const istring[] symbolChain) +{ + import std.array:empty; + + DSymbol*[] s = sc.getSymbolsByName(symbolChain[0]); + if (s.empty) + return null; + + DSymbol* sym = s[0]; + foreach (i; 1 .. symbolChain.length) + { + if (sym.kind == CompletionKind.variableName || sym.kind == CompletionKind.memberVariableName + || sym.kind == CompletionKind.functionName) + sym = sym.type; + if (sym is null) + return null; + auto p = sym.getPartsByName(symbolChain[i]); + if (p.empty) + return null; + sym = p[0]; + } + return sym; +} + struct ArgMismatch { - size_t argIndex; - size_t paramIndex; + size_t argIndex; + size_t paramIndex; + string name; } -ArgMismatch[] compareArgsToParams(const string[] params, const string[] args) pure -in +const(ArgMismatch[]) compareArgsToParams(const istring[] params, const istring[] args) pure { - assert(args.length == params.length); -} -body -{ - ArgMismatch[] retVal; - foreach (i, arg; args) - { - if (arg is null || arg == params[i]) - continue; - foreach (j, param; params) - if (param == arg) - retVal ~= ArgMismatch(i, j); - } - return retVal; + if(args.length != params.length) + return []; + ArgMismatch[] retVal; + foreach (i, arg; args) + { + if (arg is null || arg == params[i]) + continue; + foreach (j, param; params) + if (param == arg) + retVal ~= ArgMismatch(i, j, arg); + } + return retVal; } -string createWarningFromMismatch(ref const ArgMismatch mismatch, const string commonName) pure +string createWarningFromMismatch(const ArgMismatch mismatch) pure { - import std.format : format; + import std.format : format; - return "Argument %d is named '%s', but this is the name of parameter %d".format( - mismatch.argIndex + 1, commonName, mismatch.paramIndex + 1); + return "Argument %d is named '%s', but this is the name of parameter %d" + .format(mismatch.argIndex + 1, mismatch.name, mismatch.paramIndex + 1); } unittest { - { - string[] args = ["a", "b", "c"]; - string[] params = ["a", "b", "c"]; - immutable res = compareArgsToParams(params, args); - assert(res == []); - } - { - string[] args = ["a", "c", "b"]; - string[] params = ["a", "b", "c"]; - immutable res = compareArgsToParams(params, args); - assert(res == [ArgMismatch(1, 2), ArgMismatch(2, 1)]); - } - { - string[] args = ["a", "c", "b"]; - string[] params = ["alpha", "bravo", "c"]; - immutable res = compareArgsToParams(params, args); - assert(res == [ArgMismatch(1, 2)]); - } + { + string[] args = ["a", "b", "c"]; + string[] params = ["a", "b", "c"]; + immutable res = compareArgsToParams(params, args); + assert(res == []); + } + + { + string[] args = ["a", "c", "b"]; + string[] params = ["a", "b", "c"]; + immutable res = compareArgsToParams(params, args); + assert(res == [ArgMismatch(1, 2), ArgMismatch(2, 1)]); + } + + { + string[] args = ["a", "c", "b"]; + string[] params = ["alpha", "bravo", "c"]; + immutable res = compareArgsToParams(params, args); + assert(res == [ArgMismatch(1, 2)]); + } + + { + string[] args = ["a", "b"]; + string[] params = [null, "b"]; + immutable res = compareArgsToParams(params, args); + assert(res == []); + } } diff --git a/src/analysis/run.d b/src/analysis/run.d index 647e607..1aad48a 100644 --- a/src/analysis/run.d +++ b/src/analysis/run.d @@ -45,6 +45,7 @@ import analysis.local_imports; import analysis.unmodified; import analysis.if_statements; import analysis.redundant_parens; +import analysis.mismatched_args; import memory.allocators:BlockAllocator; @@ -202,6 +203,7 @@ MessageSet analyze(string fileName, const Module m, if (analysisConfig.local_import_check) checks ~= new LocalImportCheck(fileName, moduleScope); if (analysisConfig.could_be_immutable_check) checks ~= new UnmodifiedFinder(fileName, moduleScope); if (analysisConfig.redundant_parens_check) checks ~= new RedundantParenCheck(fileName, moduleScope); + if (analysisConfig.mismatched_args_check) checks ~= new MismatchedArgumentCheck(fileName, moduleScope); version(none) if (analysisConfig.redundant_if_check) checks ~= new IfStatementCheck(fileName, moduleScope); foreach (check; checks)