diff --git a/src/dscanner/analysis/lambda_return_check.d b/src/dscanner/analysis/lambda_return_check.d index 583154c..11c18d5 100644 --- a/src/dscanner/analysis/lambda_return_check.d +++ b/src/dscanner/analysis/lambda_return_check.d @@ -5,85 +5,90 @@ module dscanner.analysis.lambda_return_check; -import dparse.ast; -import dparse.lexer; import dscanner.analysis.base; -import dscanner.utils : safeAccess; +import dmd.tokens : Token, TOK; -final class LambdaReturnCheck : BaseAnalyzer +// TODO: Fix AutoFix +extern (C++) class LambdaReturnCheck(AST) : BaseAnalyzerDmd { - alias visit = BaseAnalyzer.visit; - + alias visit = BaseAnalyzerDmd.visit; mixin AnalyzerInfo!"lambda_return_check"; - this(BaseAnalyzerArguments args) + private enum KEY = "dscanner.confusing.lambda_returns_lambda"; + private enum MSG = "This lambda returns a lambda. Add parenthesis to clarify."; + + private Token[] tokens; + + extern (D) this(string fileName, bool skipTests = false) { - super(args); + super(fileName, skipTests); + lexFile(); } - override void visit(const FunctionLiteralExpression fLit) + private void lexFile() { - import std.algorithm : find; + import dscanner.utils : readFile; + import dmd.errorsink : ErrorSinkNull; + import dmd.globals : global; + import dmd.lexer : Lexer; - auto fe = safeAccess(fLit).assignExpression.as!UnaryExpression - .primaryExpression.functionLiteralExpression.unwrap; + auto bytes = readFile(fileName) ~ '\0'; + __gshared ErrorSinkNull errorSinkNull; + if (!errorSinkNull) + errorSinkNull = new ErrorSinkNull; - if (fe is null || fe.parameters !is null || fe.identifier != tok!"" || - fe.specifiedFunctionBody is null || fe.specifiedFunctionBody.blockStatement is null) - { + scope lexer = new Lexer(null, cast(char*) bytes, 0, bytes.length, 0, 0, errorSinkNull, &global.compileEnv); + while (lexer.nextToken() != TOK.endOfFile) + tokens ~= lexer.token; + } + + override void visit(AST.FuncLiteralDeclaration lambda) + { + import std.algorithm.iteration : filter; + import std.algorithm.searching : canFind, find, until; + + super.visit(lambda); + + if (lambda.fbody.isReturnStatement() is null) return; - } - auto start = &fLit.tokens[0]; - auto endIncl = &fe.specifiedFunctionBody.tokens[0]; - assert(endIncl >= start); - auto tokens = start[0 .. endIncl - start + 1]; - auto arrow = tokens.find!(a => a.type == tok!"=>"); - AutoFix[] autofixes; - if (arrow.length) - { - if (fLit.tokens[0] == tok!"(") - autofixes ~= AutoFix.replacement(arrow[0], "", "Remove arrow (use function body)"); - else - autofixes ~= AutoFix.insertionBefore(fLit.tokens[0], "(", "Remove arrow (use function body)") - .concat(AutoFix.insertionAfter(fLit.tokens[0], ")")) - .concat(AutoFix.replacement(arrow[0], "")); - } - autofixes ~= AutoFix.insertionBefore(*endIncl, "() ", "Add parenthesis (return delegate)"); - addErrorMessage(tokens, KEY, "This lambda returns a lambda. Add parenthesis to clarify.", - autofixes); + auto tokenRange = tokens.filter!(t => t.loc.fileOffset > lambda.loc.fileOffset) + .filter!(t => t.loc.fileOffset < lambda.endloc.fileOffset) + .find!(t => t.value == TOK.goesTo); + + if (!tokenRange.canFind!(t => t.value == TOK.leftCurly)) + return; + + if (!tokenRange.canFind!(t => t.value == TOK.leftParenthesis)) + addErrorMessage(cast(ulong) lambda.loc.linnum, cast(ulong) lambda.loc.charnum, KEY, MSG); } - -private: - enum KEY = "dscanner.confusing.lambda_returns_lambda"; } -version(Windows) {/*because of newline in code*/} else unittest { import dscanner.analysis.config : Check, disabledConfig, StaticAnalysisConfig; - import dscanner.analysis.helpers : assertAnalyzerWarnings, assertAutoFix; + import dscanner.analysis.helpers : assertAnalyzerWarningsDMD, assertAutoFix; import std.stdio : stderr; + import std.format : format; StaticAnalysisConfig sac = disabledConfig(); sac.lambda_return_check = Check.enabled; + auto msg = "This lambda returns a lambda. Add parenthesis to clarify."; - assertAnalyzerWarnings(q{ + assertAnalyzerWarningsDMD(q{ void main() { int[] b; - auto a = b.map!(a => { return a * a + 2; }).array(); /+ - ^^^^^^ [warn]: This lambda returns a lambda. Add parenthesis to clarify. +/ - pragma(msg, typeof(a => { return a; })); /+ - ^^^^^^ [warn]: This lambda returns a lambda. Add parenthesis to clarify. +/ - pragma(msg, typeof((a) => { return a; })); /+ - ^^^^^^^^ [warn]: This lambda returns a lambda. Add parenthesis to clarify. +/ + auto a = b.map!(a => { return a * a + 2; }).array(); // [warn]: %s + pragma(msg, typeof(a => { return a; })); // [warn]: %s + pragma(msg, typeof((a) => { return a; })); // [warn]: %s pragma(msg, typeof({ return a; })); pragma(msg, typeof(a => () { return a; })); + b.map!(a => a * 2); } - }c, sac); - + }c.format(msg, msg, msg), sac); + /+ TODO: Fix AutoFix assertAutoFix(q{ void main() { @@ -106,7 +111,7 @@ unittest pragma(msg, typeof((a) { return a; })); // fix:0 pragma(msg, typeof((a) => () { return a; })); // fix:1 } - }c, sac); + }c, sac);+/ stderr.writeln("Unittest for LambdaReturnCheck passed."); } diff --git a/src/dscanner/analysis/run.d b/src/dscanner/analysis/run.d index 684c4af..1c95a51 100644 --- a/src/dscanner/analysis/run.d +++ b/src/dscanner/analysis/run.d @@ -862,10 +862,6 @@ private BaseAnalyzer[] getAnalyzersForModuleAndConfig(string fileName, analysisConfig.long_line_check == Check.skipTests && !ut), analysisConfig.max_line_length); - if (moduleName.shouldRun!LambdaReturnCheck(analysisConfig)) - checks ~= new LambdaReturnCheck(args.setSkipTests( - analysisConfig.lambda_return_check == Check.skipTests && !ut)); - if (moduleName.shouldRun!AutoFunctionChecker(analysisConfig)) checks ~= new AutoFunctionChecker(args.setSkipTests( analysisConfig.auto_function_check == Check.skipTests && !ut)); @@ -1342,6 +1338,12 @@ MessageSet analyzeDmd(string fileName, ASTCodegen.Module m, const char[] moduleN config.label_var_same_name_check == Check.skipTests && !ut ); + if (moduleName.shouldRunDmd!(LambdaReturnCheck!ASTCodegen)(config)) + visitors ~= new LambdaReturnCheck!ASTCodegen( + fileName, + config.lambda_return_check == Check.skipTests && !ut + ); + foreach (visitor; visitors) { m.accept(visitor);