From 4268f6327f89a76ed490cd330c7ec0ce0d655a2a Mon Sep 17 00:00:00 2001 From: Vladiwostok <55026261+Vladiwostok@users.noreply.github.com> Date: Tue, 6 Aug 2024 18:49:44 +0300 Subject: [PATCH] Replace libdparse with DMD in UnusedParameterCheck (#116) * Replace libdparse with DMD in UnusedParameterCheck * Add workaround for gdc-12 compilation --- src/dscanner/analysis/run.d | 10 +- src/dscanner/analysis/unused_parameter.d | 240 +++++++++++++++++++---- 2 files changed, 210 insertions(+), 40 deletions(-) diff --git a/src/dscanner/analysis/run.d b/src/dscanner/analysis/run.d index c39510c..900d4dc 100644 --- a/src/dscanner/analysis/run.d +++ b/src/dscanner/analysis/run.d @@ -849,10 +849,6 @@ private BaseAnalyzer[] getAnalyzersForModuleAndConfig(string fileName, checks ~= new UnusedVariableCheck(args.setSkipTests( analysisConfig.unused_variable_check == Check.skipTests && !ut)); - if (moduleName.shouldRun!UnusedParameterCheck(analysisConfig)) - checks ~= new UnusedParameterCheck(args.setSkipTests( - analysisConfig.unused_parameter_check == Check.skipTests && !ut)); - if (moduleName.shouldRun!LineLengthCheck(analysisConfig)) checks ~= new LineLengthCheck(args.setSkipTests( analysisConfig.long_line_check == Check.skipTests && !ut), @@ -1350,6 +1346,12 @@ MessageSet analyzeDmd(string fileName, ASTCodegen.Module m, const char[] moduleN config.auto_function_check == Check.skipTests && !ut ); + if (moduleName.shouldRunDmd!(UnusedParameterCheck!ASTCodegen)(config)) + visitors ~= new UnusedParameterCheck!ASTCodegen( + fileName, + config.unused_parameter_check == Check.skipTests && !ut + ); + foreach (visitor; visitors) { m.accept(visitor); diff --git a/src/dscanner/analysis/unused_parameter.d b/src/dscanner/analysis/unused_parameter.d index 878b510..fb8b31a 100644 --- a/src/dscanner/analysis/unused_parameter.d +++ b/src/dscanner/analysis/unused_parameter.d @@ -4,64 +4,172 @@ // http://www.boost.org/LICENSE_1_0.txt) module dscanner.analysis.unused_parameter; -import dparse.ast; -import dparse.lexer; import dscanner.analysis.base; -import dscanner.analysis.unused; -import dsymbol.scope_ : Scope; +import dmd.astenums : STC; +import std.algorithm : all, canFind, each, filter, map; +import std.conv : to; /** - * Checks for unused variables. + * Checks for unused function parameters. */ -final class UnusedParameterCheck : UnusedStorageCheck +extern (C++) class UnusedParameterCheck(AST) : BaseAnalyzerDmd { - alias visit = UnusedStorageCheck.visit; - + alias visit = BaseAnalyzerDmd.visit; mixin AnalyzerInfo!"unused_parameter_check"; - /** - * Params: - * fileName = the name of the file being analyzed - */ - this(BaseAnalyzerArguments args) + private enum KEY = "dscanner.suspicious.unused_parameter"; + private enum MSG = "Parameter %s is never used."; + + private static struct ParamInfo { - super(args, "Parameter", "unused_parameter"); + string name; + ulong lineNum; + ulong charNum; + bool isUsed = false; } - override void visit(const Parameter parameter) + private alias ParamSet = ParamInfo[string]; + private ParamSet[] usedParams; + private bool inMixin; + + extern (D) this(string fileName, bool skipTests = false) { - import std.algorithm : among; - import std.algorithm.iteration : filter; - import std.range : empty; + super(fileName, skipTests); + pushScope(); + } - if (parameter.name != tok!"") + override void visit(AST.FuncDeclaration funcDeclaration) + { + import std.format : format; + + pushScope(); + super.visit(funcDeclaration); + + bool shouldIgnoreWarns = funcDeclaration.fbody is null || funcDeclaration.storage_class & STC.override_; + if (!shouldIgnoreWarns) + currentScope.byValue + .filter!(param => !param.isUsed) + .each!(param => addErrorMessage(param.lineNum, param.charNum, KEY, MSG.format(param.name))); + + popScope(); + } + + override void visit(AST.Parameter parameter) + { + import dmd.astenums : TY; + + if (parameter.ident is null) + return; + + auto varName = cast(string) parameter.ident.toString(); + bool shouldBeIgnored = varName.all!(c => c == '_') || parameter.storageClass & STC.ref_ + || parameter.storageClass & STC.out_ || parameter.type.ty == TY.Tpointer; + if (!shouldBeIgnored) + currentScope[varName] = ParamInfo(varName, parameter.loc.linnum, parameter.loc.charnum); + } + + override void visit(AST.TypeSArray newExp) + { + if (auto identifierExpression = newExp.dim.isIdentifierExp()) + identifierExpression.accept(this); + } + + override void visit(AST.IdentifierExp identifierExp) + { + if (identifierExp.ident) + markAsUsed(cast(string) identifierExp.ident.toString()); + + super.visit(identifierExp); + } + + mixin VisitMixin!(AST.MixinExp); + mixin VisitMixin!(AST.MixinStatement); + + private template VisitMixin(NodeType) + { + override void visit(NodeType node) { - immutable bool isRef = !parameter.parameterAttributes - .filter!(a => a.idType.among(tok!"ref", tok!"out")).empty; - immutable bool isPtr = parameter.type && !parameter.type - .typeSuffixes.filter!(a => a.star != tok!"").empty; + inMixin = true; + super.visit(node); + inMixin = false; + } + } - variableDeclared(parameter.name.text, parameter.name, isRef | isPtr); + override void visit(AST.StringExp stringExp) + { + if (!inMixin) + return; - if (parameter.default_ !is null) + string str = cast(string) stringExp.toStringz(); + currentScope.byKey + .filter!(param => canFind(str, param)) + .each!(param => markAsUsed(param)); + } + + override void visit(AST.TraitsExp traitsExp) + { + import dmd.dtemplate : isType; + + super.visit(traitsExp); + + if (traitsExp.args is null) + return; + + (*traitsExp.args).opSlice() + .map!(arg => isType(arg)) + .filter!(type => type !is null) + .map!(type => type.isTypeIdentifier()) + .filter!(typeIdentifier => typeIdentifier !is null) + .each!(typeIdentifier => markAsUsed(cast(string) typeIdentifier.toString())); + } + + private extern (D) void markAsUsed(string varName) + { + import std.range : retro; + + foreach (funcScope; usedParams.retro()) + { + if (varName in funcScope) { - interestDepth++; - parameter.default_.accept(this); - interestDepth--; + funcScope[varName].isUsed = true; + break; } } } + + @property private extern (D) ParamSet currentScope() + { + return usedParams[$ - 1]; + } + + private void pushScope() + { + // Error with gdc-12 + //usedParams ~= new ParamSet; + + // Workaround for gdc-12 + ParamSet newScope; + newScope["test"] = ParamInfo("test", 0, 0); + usedParams ~= newScope; + currentScope.remove("test"); + } + + private void popScope() + { + usedParams.length--; + } } @system unittest { import std.stdio : stderr; import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig; - import dscanner.analysis.helpers : assertAnalyzerWarnings; + import dscanner.analysis.helpers : assertAnalyzerWarningsDMD; StaticAnalysisConfig sac = disabledConfig(); sac.unused_parameter_check = Check.enabled; - assertAnalyzerWarnings(q{ + + assertAnalyzerWarningsDMD(q{ // bug encountered after correct DIP 1009 impl in dparse version (StdDdoc) @@ -71,11 +179,9 @@ final class UnusedParameterCheck : UnusedStorageCheck is(StringTypeOf!R)); } - void inPSC(in int a){} /+ - ^ [warn]: Parameter a is never used. +/ + void inPSC(in int a){} // [warn]: Parameter a is never used. - void doStuff(int a, int b) /+ - ^ [warn]: Parameter b is never used. +/ + void doStuff(int a, int b) // [warn]: Parameter b is never used. { return a; } @@ -96,8 +202,7 @@ final class UnusedParameterCheck : UnusedStorageCheck { auto cb1 = delegate(size_t _) {}; cb1(3); - auto cb2 = delegate(size_t a) {}; /+ - ^ [warn]: Parameter a is never used. +/ + auto cb2 = delegate(size_t a) {}; // [warn]: Parameter a is never used. cb2(3); } @@ -118,5 +223,68 @@ final class UnusedParameterCheck : UnusedStorageCheck } }c, sac); + + assertAnalyzerWarningsDMD(q{ + void testMixinExpression(const Declaration decl) + { + foreach (property; possibleDeclarations) + if (auto fn = mixin("decl." ~ property)) + addMessage(fn.name.type ? [fn.name] : fn.tokens, fn.name.text); + } + }c, sac); + + assertAnalyzerWarningsDMD(q{ + void testNestedFunction(int a) + { + int nestedFunction(int b) + { + return a + b; + } + + nestedFunction(5); + } + + void testNestedFunctionShadowing(int a) // [warn]: Parameter a is never used. + { + int nestedFunctionShadowing(int a) + { + return a + 5; + } + + nestedFunction(5); + } + }c, sac); + + assertAnalyzerWarningsDMD(q{ + override protected void testOverrideFunction(int a) + { + return; + } + }c, sac); + + assertAnalyzerWarningsDMD(q{ + void testRefParam(ref LogEntry payload) + { + return; + } + + void testOutParam(out LogEntry payload) + { + return; + } + + void testPointerParam(LogEntry* payload) + { + return; + } + }c, sac); + + assertAnalyzerWarningsDMD(q{ + private char[] testStaticArray(size_t size) @safe pure nothrow + { + return new char[size]; + } + }c, sac); + stderr.writeln("Unittest for UnusedParameterCheck passed."); }