From 232cd304de290509b842b87fa27d47969c275306 Mon Sep 17 00:00:00 2001 From: Vladiwostok <55026261+Vladiwostok@users.noreply.github.com> Date: Tue, 10 Dec 2024 09:25:16 +0200 Subject: [PATCH] Cleanup code and fix integration tests (#172) * Delete libdparse unit test assertion function * Delete unused libdparse visitors from base.d * Improve StaticIfElse warning location * Improve FunctionAttributeCheck warning location * Switch to DMD flow for listing autofixes * Extract dmd analyzer selection in a separate function * Make getName() method in BaseAnalyzerDmd public * Fix offsets in integration test json * Improve StyleChecker warning location * Enable integration tests in CI * Fix Autofix flow * Remove & comment dead code * Remove dead code from autofix unit test * Remove dead code * Remove dead code from autofix.d * Clean up code in helpers.d * Clean up code in run.d and migrate StatsCollector to dmd * Fix reading code from stdin * Return if errors are found in analysis flows * Remove dead code * Check for Windows line terminators in integration tests --- .github/workflows/default.yml | 15 +- src/dscanner/analysis/always_curly.d | 10 +- src/dscanner/analysis/auto_function.d | 2 +- src/dscanner/analysis/autofix.d | 155 ++---- src/dscanner/analysis/base.d | 479 +----------------- src/dscanner/analysis/del.d | 7 +- src/dscanner/analysis/enumarrayliteral.d | 2 +- .../analysis/explicitly_annotated_unittests.d | 13 +- src/dscanner/analysis/final_attribute.d | 2 +- src/dscanner/analysis/function_attributes.d | 17 +- src/dscanner/analysis/helpers.d | 253 +-------- src/dscanner/analysis/lambda_return_check.d | 2 +- src/dscanner/analysis/length_subtraction.d | 2 +- src/dscanner/analysis/run.d | 249 ++------- src/dscanner/analysis/rundmd.d | 28 +- src/dscanner/analysis/static_if_else.d | 16 +- src/dscanner/analysis/stats_collector.d | 170 +++++-- src/dscanner/analysis/style.d | 38 +- src/dscanner/main.d | 12 +- src/dscanner/reports.d | 2 +- tests/it.sh | 7 +- .../autofix_ide/source_autofix.autofix.json | 24 +- .../it/autofix_ide/source_autofix.report.json | 75 ++- .../source_autofix_windows.report.json | 139 +++++ 24 files changed, 561 insertions(+), 1158 deletions(-) create mode 100644 tests/it/autofix_ide/source_autofix_windows.report.json diff --git a/.github/workflows/default.yml b/.github/workflows/default.yml index 413eaa7..e02b96e 100644 --- a/.github/workflows/default.yml +++ b/.github/workflows/default.yml @@ -168,11 +168,16 @@ jobs: fi "./bin/dscanner$EXE" --styleCheck -f "$FORMAT" src - # TODO: fixme - #- name: Integration Tests - #run: ./it.sh - #working-directory: tests - #shell: bash + - name: Integration Tests +# run: ./it.sh + run: | + if [ "$RUNNER_OS" == "Windows" ]; then + ./it.sh Windows + else + ./it.sh Unix + fi + working-directory: tests + shell: bash - name: Run style checks if: ${{ matrix.compiler.dmd == 'dmd' && matrix.build.type == 'make' }} diff --git a/src/dscanner/analysis/always_curly.d b/src/dscanner/analysis/always_curly.d index 2783c89..3e13914 100644 --- a/src/dscanner/analysis/always_curly.d +++ b/src/dscanner/analysis/always_curly.d @@ -133,7 +133,7 @@ extern (C++) class AlwaysCurlyCheck(AST) : BaseAnalyzerDmd unittest { import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig; - import dscanner.analysis.helpers : assertAnalyzerWarningsDMD, assertAutoFix; + import dscanner.analysis.helpers : assertAnalyzerWarningsDMD; import std.stdio : stderr; StaticAnalysisConfig sac = disabledConfig(); @@ -240,7 +240,7 @@ unittest void test() { if(true) { return; } // fix:0 } - }c, sac, true); + }c, sac); assertAutoFix(q{ void test() { @@ -250,7 +250,7 @@ unittest void test() { foreach(_; 0 .. 10 ) { return; } // fix:0 } - }c, sac, true); + }c, sac); assertAutoFix(q{ void test() { @@ -260,7 +260,7 @@ unittest void test() { for(int i = 0; i < 10; ++i) { return; } // fix:0 } - }c, sac, true); + }c, sac); assertAutoFix(q{ void test() { @@ -270,7 +270,7 @@ unittest void test() { do { return; } while(true); // fix:0 } - }c, sac, true); + }c, sac); stderr.writeln("Unittest for AutoFix AlwaysCurly passed."); diff --git a/src/dscanner/analysis/auto_function.d b/src/dscanner/analysis/auto_function.d index f2dbe6e..4588621 100644 --- a/src/dscanner/analysis/auto_function.d +++ b/src/dscanner/analysis/auto_function.d @@ -226,7 +226,7 @@ unittest @safe void doStuff(){} // fix @Custom void doStuff(){} // fix - }c, sac, true); + }c, sac); stderr.writeln("Unittest for AutoFunctionChecker passed."); } diff --git a/src/dscanner/analysis/autofix.d b/src/dscanner/analysis/autofix.d index d152945..1f8f2f8 100644 --- a/src/dscanner/analysis/autofix.d +++ b/src/dscanner/analysis/autofix.d @@ -2,124 +2,23 @@ module dscanner.analysis.autofix; import std.algorithm : filter, findSplit; import std.conv : to; +import std.file : exists, remove; import std.functional : toDelegate; import std.stdio; -import dparse.lexer; -import dparse.rollback_allocator; -import dparse.ast : Module; - -import dsymbol.modulecache : ModuleCache; - -import dscanner.analysis.base : AutoFix, AutoFixFormatting, BaseAnalyzer, Message; +import dscanner.analysis.base : AutoFix, AutoFixFormatting, BaseAnalyzer, BaseAnalyzerDmd, Message; import dscanner.analysis.config : StaticAnalysisConfig; import dscanner.analysis.run : analyze, doNothing; -import dscanner.utils : readFile, readStdin; - -private void resolveAutoFixes( - ref Message message, - string fileName, - ref ModuleCache moduleCache, - scope const(Token)[] tokens, - const Module m, - const StaticAnalysisConfig analysisConfig, - const AutoFixFormatting overrideFormattingConfig = AutoFixFormatting.invalid -) -{ - resolveAutoFixes(message.checkName, message.autofixes, fileName, moduleCache, - tokens, m, analysisConfig, overrideFormattingConfig); -} - -private void resolveAutoFixes(string messageCheckName, AutoFix[] autofixes, string fileName, - ref ModuleCache moduleCache, - scope const(Token)[] tokens, const Module m, - const StaticAnalysisConfig analysisConfig, - const AutoFixFormatting overrideFormattingConfig = AutoFixFormatting.invalid) -{ - import core.memory : GC; - import dsymbol.conversion.first : FirstPass; - import dsymbol.conversion.second : secondPass; - import dsymbol.scope_ : Scope; - import dsymbol.semantic : SemanticSymbol; - import dsymbol.string_interning : internString; - import dsymbol.symbol : DSymbol; - import dscanner.analysis.run : getAnalyzersForModuleAndConfig; - - const(AutoFixFormatting) formattingConfig = - overrideFormattingConfig is AutoFixFormatting.invalid - ? analysisConfig.getAutoFixFormattingConfig() - : overrideFormattingConfig; - - scope first = new FirstPass(m, internString(fileName), &moduleCache, null); - first.run(); - - secondPass(first.rootSymbol, first.moduleScope, moduleCache); - auto moduleScope = first.moduleScope; - scope(exit) typeid(DSymbol).destroy(first.rootSymbol.acSymbol); - scope(exit) typeid(SemanticSymbol).destroy(first.rootSymbol); - scope(exit) typeid(Scope).destroy(first.moduleScope); - - GC.disable; - scope (exit) - GC.enable; - - foreach (BaseAnalyzer check; getAnalyzersForModuleAndConfig(fileName, tokens, m, analysisConfig, moduleScope)) - { - if (check.getName() == messageCheckName) - { - foreach (ref autofix; autofixes) - autofix.resolveAutoFixFromCheck(check, m, tokens, formattingConfig); - return; - } - } - - throw new Exception("Cannot find analyzer " ~ messageCheckName - ~ " to resolve autofix with."); -} - -void resolveAutoFixFromCheck( - ref AutoFix autofix, - BaseAnalyzer check, - const Module m, - scope const(Token)[] tokens, - const AutoFixFormatting formattingConfig -) -{ - import std.sumtype : match; - - autofix.replacements.match!( - (AutoFix.ResolveContext context) { - autofix.replacements = check.resolveAutoFix(m, tokens, context, formattingConfig); - }, - (_) {} - ); -} - -private AutoFix.CodeReplacement[] resolveAutoFix(string messageCheckName, AutoFix.ResolveContext context, - string fileName, - ref ModuleCache moduleCache, - scope const(Token)[] tokens, const Module m, - const StaticAnalysisConfig analysisConfig, - const AutoFixFormatting overrideFormattingConfig = AutoFixFormatting.invalid) -{ - AutoFix temp; - temp.replacements = context; - resolveAutoFixes(messageCheckName, (&temp)[0 .. 1], fileName, moduleCache, - tokens, m, analysisConfig, overrideFormattingConfig); - return temp.expectReplacements("resolving didn't work?!"); -} +import dscanner.analysis.rundmd; +import dscanner.utils : getModuleName, readFile, readStdin; void listAutofixes( StaticAnalysisConfig config, string resolveMessage, bool usingStdin, - string fileName, - StringCache* cache, - ref ModuleCache moduleCache + string fileName ) { - import dparse.parser : parseModule; - import dscanner.analysis.base : Message; import std.format : format; import std.json : JSONValue; @@ -145,30 +44,35 @@ void listAutofixes( bool matchesCursor(Message m) { - return isBytes - ? req.bytes >= m.startIndex && req.bytes <= m.endIndex - : req.line >= m.startLine && req.line <= m.endLine - && (req.line > m.startLine || req.column >= m.startColumn) - && (req.line < m.endLine || req.column <= m.endColumn); + return isBytes ? req.bytes >= m.startIndex && req.bytes <= m.endIndex + : req.line >= m.startLine && req.line <= m.endLine + && (req.line > m.startLine || req.column >= m.startColumn) + && (req.line < m.endLine || req.column <= m.endColumn); } - RollbackAllocator rba; - LexerConfig lexerConfig; - lexerConfig.fileName = fileName; - lexerConfig.stringBehavior = StringBehavior.source; - auto tokens = getTokensForParser(usingStdin ? readStdin() - : readFile(fileName), lexerConfig, cache); - auto mod = parseModule(tokens, fileName, &rba, toDelegate(&doNothing)); + ubyte[] code; + if (usingStdin) + { + code = readStdin(); + fileName = "stdin.d"; + File f = File(fileName, "w"); + f.rawWrite(code); + f.close(); + } + else + { + code = readFile(fileName); + } - auto messages = analyze(fileName, mod, config, moduleCache, tokens); + auto dmdModule = parseDmdModule(fileName, cast(string) code); + auto moduleName = getModuleName(dmdModule.md); + auto messages = analyzeDmd(fileName, dmdModule, moduleName, config); with (stdout.lockingTextWriter) { put("["); foreach (message; messages[].filter!matchesCursor) { - resolveAutoFixes(message, fileName, moduleCache, tokens, mod, config); - foreach (i, autofix; message.autofixes) { put(i == 0 ? "\n" : ",\n"); @@ -191,6 +95,12 @@ void listAutofixes( put("\n]"); } stdout.flush(); + + if (usingStdin) + { + assert(exists(fileName)); + remove(fileName); + } } void improveAutoFixWhitespace(scope const(char)[] code, AutoFix.CodeReplacement[] replacements) @@ -227,7 +137,8 @@ void improveAutoFixWhitespace(scope const(char)[] code, AutoFix.CodeReplacement[ { assert(replacement.range[0] >= 0 && replacement.range[0] < code.length && replacement.range[1] >= 0 && replacement.range[1] < code.length - && replacement.range[0] <= replacement.range[1], "trying to autofix whitespace on code that doesn't match with what the replacements were generated for"); + && replacement.range[0] <= replacement.range[1], + "trying to autofix whitespace on code that doesn't match with what the replacements were generated for"); void growRight() { diff --git a/src/dscanner/analysis/base.d b/src/dscanner/analysis/base.d index 70a5c80..8fffd24 100644 --- a/src/dscanner/analysis/base.d +++ b/src/dscanner/analysis/base.d @@ -82,31 +82,6 @@ struct AutoFix return ret; } - static AutoFix replacement(const Token token, string newText, string name = null) - { - if (!name.length) - { - auto text = token.text.length ? token.text : str(token.type); - if (newText.length) - name = "Replace `" ~ text ~ "` with `" ~ newText ~ "`"; - else - name = "Remove `" ~ text ~ "`"; - } - return replacement([token], newText, name); - } - - static AutoFix replacement(const BaseNode node, string newText, string name) - { - return replacement(node.tokens, newText, name); - } - - static AutoFix replacement(const Token[] tokens, string newText, string name) - in(tokens.length > 0, "must provide at least one token") - { - auto end = tokens[$ - 1].text.length ? tokens[$ - 1].text : str(tokens[$ - 1].type); - return replacement([tokens[0].index, tokens[$ - 1].index + end.length], newText, name); - } - static AutoFix replacement(size_t[2] range, string newText, string name) { AutoFix ret; @@ -117,17 +92,6 @@ struct AutoFix return ret; } - static AutoFix insertionBefore(const Token token, string content, string name = null) - { - return insertionAt(token.index, content, name); - } - - static AutoFix insertionAfter(const Token token, string content, string name = null) - { - auto tokenText = token.text.length ? token.text : str(token.type); - return insertionAt(token.index + tokenText.length, content, name); - } - static AutoFix insertionAt(size_t index, string content, string name = null) { assert(content.length > 0, "generated auto fix inserting text without content"); @@ -143,24 +107,6 @@ struct AutoFix return ret; } - static AutoFix indentLines(scope const(Token)[] tokens, const AutoFixFormatting formatting, string name = "Indent code") - { - CodeReplacement[] inserts; - size_t line = -1; - foreach (token; tokens) - { - if (line != token.line) - { - line = token.line; - inserts ~= CodeReplacement([token.index, token.index], formatting.indentation); - } - } - AutoFix ret; - ret.name = name; - ret.replacements = inserts; - return ret; - } - AutoFix concat(AutoFix other) const { import std.algorithm : sort; @@ -291,33 +237,6 @@ struct Message deprecated("Use startLine instead") alias line = startLine; deprecated("Use startColumn instead") alias column = startColumn; - static Diagnostic from(string fileName, const BaseNode node, string message) - { - return from(fileName, node !is null ? node.tokens : [], message); - } - - static Diagnostic from(string fileName, const Token token, string message) - { - auto text = token.text.length ? token.text : str(token.type); - return from(fileName, - [token.index, token.index + text.length], - token.line, - [token.column, token.column + text.length], - message); - } - - static Diagnostic from(string fileName, const Token[] tokens, string message) - { - auto start = tokens.length ? tokens[0] : Token.init; - auto end = tokens.length ? tokens[$ - 1] : Token.init; - auto endText = end.text.length ? end.text : str(end.type); - return from(fileName, - [start.index, end.index + endText.length], - [start.line, end.line], - [start.column, end.column + endText.length], - message); - } - static Diagnostic from(string fileName, size_t[2] index, size_t line, size_t[2] columns, string message) { return Message.Diagnostic(fileName, index[0], index[1], line, line, columns[0], columns[1], message); @@ -342,7 +261,7 @@ struct Message /// the `BaseAnalyzer.resolveAutoFix` method with. AutoFix[] autofixes; - deprecated this(string fileName, size_t line, size_t column, string key = null, string message = null, string checkName = null) + this(string fileName, size_t line, size_t column, string key = null, string message = null, string checkName = null) { diagnostic.fileName = fileName; diagnostic.startLine = diagnostic.endLine = line; @@ -395,7 +314,7 @@ mixin template AnalyzerInfo(string checkName) { enum string name = checkName; - extern(D) override protected string getName() + extern (D) override protected string getName() { return name; } @@ -503,48 +422,6 @@ protected: } } - deprecated("Use the overload taking start and end locations or a Node instead") - void addErrorMessage(size_t line, size_t column, string key, string message) - { - _messages.insert(Message(fileName, line, column, key, message, getName())); - } - - void addErrorMessage(const BaseNode node, string key, string message, AutoFix[] autofixes = null) - { - addErrorMessage(Message.Diagnostic.from(fileName, node, message), key, autofixes); - } - - void addErrorMessage(const Token token, string key, string message, AutoFix[] autofixes = null) - { - addErrorMessage(Message.Diagnostic.from(fileName, token, message), key, autofixes); - } - - void addErrorMessage(const Token[] tokens, string key, string message, AutoFix[] autofixes = null) - { - addErrorMessage(Message.Diagnostic.from(fileName, tokens, message), key, autofixes); - } - - void addErrorMessage(size_t[2] index, size_t line, size_t[2] columns, string key, string message, AutoFix[] autofixes = null) - { - addErrorMessage(index, [line, line], columns, key, message, autofixes); - } - - void addErrorMessage(size_t[2] index, size_t[2] lines, size_t[2] columns, string key, string message, AutoFix[] autofixes = null) - { - auto d = Message.Diagnostic.from(fileName, index, lines, columns, message); - _messages.insert(Message(d, key, getName(), autofixes)); - } - - void addErrorMessage(Message.Diagnostic diagnostic, string key, AutoFix[] autofixes = null) - { - _messages.insert(Message(diagnostic, key, getName(), autofixes)); - } - - void addErrorMessage(Message.Diagnostic diagnostic, Message.Diagnostic[] supplemental, string key, AutoFix[] autofixes = null) - { - _messages.insert(Message(diagnostic, supplemental, key, getName(), autofixes)); - } - /** * The file name */ @@ -555,343 +432,6 @@ protected: MessageSet _messages; } -/// Find the token with the given type, otherwise returns the whole range or a user-specified fallback, if set. -const(Token)[] findTokenForDisplay(const BaseNode node, IdType type, const(Token)[] fallback = null) -{ - return node.tokens.findTokenForDisplay(type, fallback); -} -/// ditto -const(Token)[] findTokenForDisplay(const Token[] tokens, IdType type, const(Token)[] fallback = null) -{ - foreach (i, token; tokens) - if (token.type == type) - return tokens[i .. i + 1]; - return fallback is null ? tokens : fallback; -} - -abstract class ScopedBaseAnalyzer : BaseAnalyzer -{ -public: - this(BaseAnalyzerArguments args) - { - super(args); - } - - - template ScopedVisit(NodeType) - { - override void visit(const NodeType n) - { - pushScopeImpl(); - scope (exit) - popScopeImpl(); - n.accept(this); - } - } - - alias visit = BaseAnalyzer.visit; - - mixin ScopedVisit!BlockStatement; - mixin ScopedVisit!ForeachStatement; - mixin ScopedVisit!ForStatement; - mixin ScopedVisit!Module; - mixin ScopedVisit!StructBody; - mixin ScopedVisit!TemplateDeclaration; - mixin ScopedVisit!WithStatement; - mixin ScopedVisit!WhileStatement; - mixin ScopedVisit!DoStatement; - // mixin ScopedVisit!SpecifiedFunctionBody; // covered by BlockStatement - mixin ScopedVisit!ShortenedFunctionBody; - - override void visit(const SwitchStatement switchStatement) - { - switchStack.length++; - scope (exit) - switchStack.length--; - switchStatement.accept(this); - } - - override void visit(const IfStatement ifStatement) - { - pushScopeImpl(); - if (ifStatement.condition) - ifStatement.condition.accept(this); - if (ifStatement.thenStatement) - ifStatement.thenStatement.accept(this); - popScopeImpl(); - - if (ifStatement.elseStatement) - { - pushScopeImpl(); - ifStatement.elseStatement.accept(this); - popScopeImpl(); - } - } - - static foreach (T; AliasSeq!(CaseStatement, DefaultStatement, CaseRangeStatement)) - override void visit(const T stmt) - { - // case and default statements always open new scopes and close - // previous case scopes - bool close = switchStack.length && switchStack[$ - 1].inCase; - bool b = switchStack[$ - 1].inCase; - switchStack[$ - 1].inCase = true; - scope (exit) - switchStack[$ - 1].inCase = b; - if (close) - { - popScope(); - pushScope(); - stmt.accept(this); - } - else - { - pushScope(); - stmt.accept(this); - popScope(); - } - } - -protected: - /// Called on new scopes, which includes for example: - /// - /// - `module m; /* here, entire file */` - /// - `{ /* here */ }` - /// - `if () { /* here */ } else { /* here */ }` - /// - `foreach (...) { /* here */ }` - /// - `case 1: /* here */ break;` - /// - `case 1: /* here, up to next case */ goto case; case 2: /* here 2 */ break;` - /// - `default: /* here */ break;` - /// - `struct S { /* here */ }` - /// - /// But doesn't include: - /// - /// - `static if (x) { /* not a separate scope */ }` (use `mixin ScopedVisit!ConditionalDeclaration;`) - /// - /// You can `mixin ScopedVisit!NodeType` to automatically call push/popScope - /// on occurences of that NodeType. - abstract void pushScope(); - /// ditto - abstract void popScope(); - - void pushScopeImpl() - { - if (switchStack.length) - switchStack[$ - 1].scopeDepth++; - pushScope(); - } - - void popScopeImpl() - { - if (switchStack.length) - switchStack[$ - 1].scopeDepth--; - popScope(); - } - - struct SwitchStack - { - int scopeDepth; - bool inCase; - } - - SwitchStack[] switchStack; -} - -unittest -{ - import core.exception : AssertError; - import dparse.lexer : getTokensForParser, LexerConfig, StringCache; - import dparse.parser : parseModule; - import dparse.rollback_allocator : RollbackAllocator; - import std.conv : to; - import std.exception : assertThrown; - - // test where we can: - // call `depth(1);` to check that the scope depth is at 1 - // if calls are syntactically not valid, define `auto depth = 1;` - // - // call `isNewScope();` to check that the scope hasn't been checked with isNewScope before - // if calls are syntactically not valid, define `auto isNewScope = void;` - // - // call `isOldScope();` to check that the scope has already been checked with isNewScope - // if calls are syntactically not valid, define `auto isOldScope = void;` - - class TestScopedAnalyzer : ScopedBaseAnalyzer - { - this(size_t codeLine) - { - super(BaseAnalyzerArguments("stdin")); - - this.codeLine = codeLine; - } - - override void visit(const FunctionCallExpression f) - { - int depth = cast(int) stack.length; - if (f.unaryExpression && f.unaryExpression.primaryExpression - && f.unaryExpression.primaryExpression.identifierOrTemplateInstance) - { - auto fname = f.unaryExpression.primaryExpression.identifierOrTemplateInstance.identifier.text; - if (fname == "depth") - { - assert(f.arguments.tokens.length == 3); - auto expected = f.arguments.tokens[1].text.to!int; - assert(expected == depth, "Expected depth=" - ~ expected.to!string ~ " in line " ~ (codeLine + f.tokens[0].line).to!string - ~ ", but got depth=" ~ depth.to!string); - } - else if (fname == "isNewScope") - { - assert(!stack[$ - 1]); - stack[$ - 1] = true; - } - else if (fname == "isOldScope") - { - assert(stack[$ - 1]); - } - } - } - - override void visit(const AutoDeclarationPart p) - { - int depth = cast(int) stack.length; - - if (p.identifier.text == "depth") - { - assert(p.initializer.tokens.length == 1); - auto expected = p.initializer.tokens[0].text.to!int; - assert(expected == depth, "Expected depth=" - ~ expected.to!string ~ " in line " ~ (codeLine + p.tokens[0].line).to!string - ~ ", but got depth=" ~ depth.to!string); - } - else if (p.identifier.text == "isNewScope") - { - assert(!stack[$ - 1]); - stack[$ - 1] = true; - } - else if (p.identifier.text == "isOldScope") - { - assert(stack[$ - 1]); - } - } - - override void pushScope() - { - stack.length++; - } - - override void popScope() - { - stack.length--; - } - - alias visit = ScopedBaseAnalyzer.visit; - - bool[] stack; - size_t codeLine; - } - - void testScopes(string code, size_t codeLine = __LINE__ - 1) - { - StringCache cache = StringCache(4096); - LexerConfig config; - RollbackAllocator rba; - auto tokens = getTokensForParser(code, config, &cache); - Module m = parseModule(tokens, "stdin", &rba); - - auto analyzer = new TestScopedAnalyzer(codeLine); - analyzer.visit(m); - } - - testScopes(q{ - auto isNewScope = void; - auto depth = 1; - auto isOldScope = void; - }); - - assertThrown!AssertError(testScopes(q{ - auto isNewScope = void; - auto isNewScope = void; - })); - - assertThrown!AssertError(testScopes(q{ - auto isOldScope = void; - })); - - assertThrown!AssertError(testScopes(q{ - auto depth = 2; - })); - - testScopes(q{ - auto isNewScope = void; - auto depth = 1; - - void foo() { - isNewScope(); - isOldScope(); - depth(2); - switch (a) - { - case 1: - isNewScope(); - depth(4); - break; - depth(4); - isOldScope(); - case 2: - isNewScope(); - depth(4); - if (a) - { - isNewScope(); - depth(6); - default: - isNewScope(); - depth(6); // since cases/default opens new scope - break; - case 3: - isNewScope(); - depth(6); // since cases/default opens new scope - break; - default: - isNewScope(); - depth(6); // since cases/default opens new scope - break; - } - break; - depth(4); - default: - isNewScope(); - depth(4); - break; - depth(4); - } - - isOldScope(); - depth(2); - - switch (a) - { - isNewScope(); - depth(3); - isOldScope(); - default: - isNewScope(); - depth(4); - break; - isOldScope(); - case 1: - isNewScope(); - depth(4); - break; - isOldScope(); - } - } - - auto isOldScope = void; - }); -} - /** * Visitor that implements the AST traversal logic. * Supports collecting error messages @@ -911,7 +451,7 @@ extern(C++) class BaseAnalyzerDmd : SemanticTimeTransitiveVisitor * Ensures that template AnalyzerInfo is instantiated in all classes * deriving from this class */ - extern(D) protected string getName() + extern(D) string getName() { assert(0); } @@ -940,6 +480,19 @@ protected: _messages.insert(Message(fileName, line, column, key, message, getName(), autofixes)); } + extern (D) void addErrorMessage(size_t[2] index, size_t[2] lines, size_t[2] columns, string key, string message) + { + auto diag = Message.Diagnostic.from(fileName, index, lines, columns, message); + _messages.insert(Message(diag, key, getName())); + } + + extern (D) void addErrorMessage(size_t[2] index, size_t[2] lines, size_t[2] columns, + string key, string message, AutoFix[] autofixes) + { + auto diag = Message.Diagnostic.from(fileName, index, lines, columns, message); + _messages.insert(Message(diag, key, getName(), autofixes)); + } + extern (D) bool skipTests; /** diff --git a/src/dscanner/analysis/del.d b/src/dscanner/analysis/del.d index a5b79a1..fe3203a 100644 --- a/src/dscanner/analysis/del.d +++ b/src/dscanner/analysis/del.d @@ -5,9 +5,7 @@ module dscanner.analysis.del; -import std.stdio; import dscanner.analysis.base; -import dscanner.analysis.helpers; /** * Checks for use of the deprecated 'delete' keyword @@ -45,7 +43,8 @@ extern(C++) class DeleteCheck(AST) : BaseAnalyzerDmd unittest { import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig; - import dscanner.analysis.helpers : assertAnalyzerWarnings, assertAutoFix; + import dscanner.analysis.helpers : assertAnalyzerWarningsDMD, assertAutoFix; + import std.stdio : stderr; StaticAnalysisConfig sac = disabledConfig(); sac.delete_check = Check.enabled; @@ -79,7 +78,7 @@ unittest auto a = new Class(); destroy(a); // fix } - }c, sac, true); + }c, sac); stderr.writeln("Unittest for DeleteCheck passed."); } diff --git a/src/dscanner/analysis/enumarrayliteral.d b/src/dscanner/analysis/enumarrayliteral.d index a4fc5bc..cad15be 100644 --- a/src/dscanner/analysis/enumarrayliteral.d +++ b/src/dscanner/analysis/enumarrayliteral.d @@ -58,7 +58,7 @@ unittest enum x = [1, 2, 3]; // fix }c, q{ static immutable x = [1, 2, 3]; // fix - }c, sac, true); + }c, sac); stderr.writeln("Unittest for EnumArrayLiteralCheck passed."); } diff --git a/src/dscanner/analysis/explicitly_annotated_unittests.d b/src/dscanner/analysis/explicitly_annotated_unittests.d index 7734e3d..b80b126 100644 --- a/src/dscanner/analysis/explicitly_annotated_unittests.d +++ b/src/dscanner/analysis/explicitly_annotated_unittests.d @@ -5,15 +5,14 @@ module dscanner.analysis.explicitly_annotated_unittests; import dscanner.analysis.base; -import dscanner.analysis.helpers; /** * Requires unittests to be explicitly annotated with either @safe or @system */ -extern(C++) class ExplicitlyAnnotatedUnittestCheck(AST) : BaseAnalyzerDmd +extern (C++) class ExplicitlyAnnotatedUnittestCheck(AST) : BaseAnalyzerDmd { - mixin AnalyzerInfo!"explicitly_annotated_unittests"; alias visit = BaseAnalyzerDmd.visit; + mixin AnalyzerInfo!"explicitly_annotated_unittests"; extern(D) this(string fileName) { @@ -46,10 +45,10 @@ private: unittest { + import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig; + import dscanner.analysis.helpers : assertAnalyzerWarningsDMD, assertAutoFix; import std.stdio : stderr; import std.format : format; - import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig; - import dscanner.analysis.helpers : assertAnalyzerWarnings; StaticAnalysisConfig sac = disabledConfig(); sac.explicitly_annotated_unittests = Check.enabled; @@ -78,7 +77,7 @@ unittest } }c, sac); - //// nested + // nested assertAutoFix(q{ unittest {} // fix:0 pure nothrow @nogc unittest {} // fix:0 @@ -97,7 +96,7 @@ unittest @system unittest {} // fix:1 pure nothrow @nogc @system unittest {} // fix:1 } - }c, sac, true); + }c, sac); stderr.writeln("Unittest for ExplicitlyAnnotatedUnittestCheck passed."); } diff --git a/src/dscanner/analysis/final_attribute.d b/src/dscanner/analysis/final_attribute.d index e1a5d98..0189314 100644 --- a/src/dscanner/analysis/final_attribute.d +++ b/src/dscanner/analysis/final_attribute.d @@ -487,7 +487,7 @@ extern (C++) class FinalAttributeChecker(AST) : BaseAnalyzerDmd static{void foo(){}} // fix void foo(){} } - }, sac, true); + }, sac); stderr.writeln("Unittest for FinalAttributeChecker passed."); } diff --git a/src/dscanner/analysis/function_attributes.d b/src/dscanner/analysis/function_attributes.d index 078fa9b..f74601d 100644 --- a/src/dscanner/analysis/function_attributes.d +++ b/src/dscanner/analysis/function_attributes.d @@ -98,8 +98,13 @@ extern (C++) class FunctionAttributeCheck(AST) : BaseAnalyzerDmd if (fd.type is null) return; - immutable ulong lineNum = cast(ulong) fd.loc.linnum; - immutable ulong charNum = cast(ulong) fd.loc.charnum; + string funcName = fd.ident is null ? "" : cast(string) fd.ident.toString(); + ulong fileOffset = cast(ulong) fd.loc.fileOffset; + ulong lineNum = cast(ulong) fd.loc.linnum; + ulong charNum = cast(ulong) fd.loc.charnum; + ulong[2] index = [fileOffset, fileOffset + funcName.length]; + ulong[2] lines = [lineNum, lineNum]; + ulong[2] columns = [charNum, charNum + funcName.length]; if (inInterface) { @@ -114,7 +119,7 @@ extern (C++) class FunctionAttributeCheck(AST) : BaseAnalyzerDmd .front.loc.fileOffset; addErrorMessage( - lineNum, charNum, KEY, ABSTRACT_MSG, + index, lines, columns, KEY, ABSTRACT_MSG, [AutoFix.replacement(offset, offset + 8, "", "Remove `abstract` attribute")] ); @@ -146,7 +151,7 @@ extern (C++) class FunctionAttributeCheck(AST) : BaseAnalyzerDmd if (!isStatic && isZeroParamProperty && !propertyRange.empty) addErrorMessage( - lineNum, charNum, KEY, CONST_MSG, + index, lines, columns, KEY, CONST_MSG, [ AutoFix.insertionAt(propertyRange.front.loc.fileOffset, "const "), AutoFix.insertionAt(propertyRange.front.loc.fileOffset, "inout "), @@ -192,7 +197,7 @@ extern (C++) class FunctionAttributeCheck(AST) : BaseAnalyzerDmd constLikeToken.loc.fileOffset + modifier.length, "(", "Make return type" ~ modifier) .concat(AutoFix.insertionAt(parensToken.loc.fileOffset - 1, ")")); - addErrorMessage(lineNum, charNum, KEY, RETURN_MSG.format(storageTok), [fix1, fix2]); + addErrorMessage(index, lines, columns, KEY, RETURN_MSG.format(storageTok), [fix1, fix2]); } } } @@ -316,7 +321,7 @@ unittest int method(); // fix } - }c, sac, true); + }c, sac); stderr.writeln("Unittest for ObjectConstCheck passed."); } diff --git a/src/dscanner/analysis/helpers.d b/src/dscanner/analysis/helpers.d index d6a8ff8..761efcf 100644 --- a/src/dscanner/analysis/helpers.d +++ b/src/dscanner/analysis/helpers.d @@ -6,6 +6,8 @@ module dscanner.analysis.helpers; import core.exception : AssertError; +import std.file : exists, remove; +import std.path : dirName; import std.stdio; import std.string; import std.traits; @@ -13,12 +15,12 @@ import std.traits; import dparse.ast; import dparse.lexer : tok, Token; import dparse.rollback_allocator; + import dscanner.analysis.base; import dscanner.analysis.config; import dscanner.analysis.run; -import dsymbol.modulecache : ModuleCache; -import std.experimental.allocator; -import std.experimental.allocator.mallocator; +import dscanner.analysis.rundmd; +import dscanner.utils : getModuleName; import dmd.astbase : ASTBase; import dmd.astcodegen; @@ -46,179 +48,6 @@ S after(S)(S value, S separator) if (isSomeString!S) return value[i + separator.length .. $]; } -string getLineIndentation(scope const(Token)[] tokens, size_t line, const AutoFixFormatting formatting) -{ - import std.algorithm : countUntil; - import std.array : array; - import std.range : repeat; - import std.string : lastIndexOfAny; - - auto idx = tokens.countUntil!(a => a.line == line); - if (idx == -1 || tokens[idx].column <= 1 || !formatting.indentation.length) - return ""; - - auto indent = tokens[idx].column - 1; - if (formatting.indentation[0] == '\t') - return (cast(immutable)'\t').repeat(indent).array; - else - return (cast(immutable)' ').repeat(indent).array; -} - -/** - * This assert function will analyze the passed in code, get the warnings, - * and make sure they match the warnings in the comments. Warnings are - * marked like so if range doesn't matter: // [warn]: Failed to do somethings. - * - * To test for start and end column, mark warnings as multi-line comments like - * this: /+ - * ^^^^^ [warn]: Failed to do somethings. +/ - */ -void assertAnalyzerWarnings(string code, const StaticAnalysisConfig config, - string file = __FILE__, size_t line = __LINE__) -{ - import dscanner.analysis.run : parseModule; - import dparse.lexer : StringCache, Token; - - StringCache cache = StringCache(StringCache.defaultBucketCount); - RollbackAllocator r; - const(Token)[] tokens; - const(Module) m = parseModule(file, cast(ubyte[]) code, &r, defaultErrorFormat, cache, false, tokens); - - ModuleCache moduleCache; - - // Run the code and get any warnings - MessageSet rawWarnings = analyze("test", m, config, moduleCache, tokens); - string[] codeLines = code.splitLines(); - - struct FoundWarning - { - string msg; - size_t startColumn, endColumn; - } - - // Get the warnings ordered by line - FoundWarning[size_t] warnings; - foreach (rawWarning; rawWarnings[]) - { - // Skip the warning if it is on line zero - immutable size_t rawLine = rawWarning.endLine; - if (rawLine == 0) - { - stderr.writefln("!!! Skipping warning because it is on line zero:\n%s", - rawWarning.message); - continue; - } - - size_t warnLine = line - 1 + rawLine; - warnings[warnLine] = FoundWarning( - format("[warn]: %s", rawWarning.message), - rawWarning.startLine != rawWarning.endLine ? 1 : rawWarning.startColumn, - rawWarning.endColumn, - ); - } - - // Get all the messages from the comments in the code - FoundWarning[size_t] messages; - bool lastLineStartedComment = false; - foreach (i, codeLine; codeLines) - { - scope (exit) - lastLineStartedComment = codeLine.stripRight.endsWith("/+", "/*") > 0; - - // Get the line of this code line - size_t lineNo = i + line; - - if (codeLine.stripLeft.startsWith("^") && lastLineStartedComment) - { - auto start = codeLine.indexOf("^") + 1; - assert(start != 0); - auto end = codeLine.indexOfNeither("^", start) + 1; - assert(end != 0); - auto warn = codeLine.indexOf("[warn]:"); - assert(warn != -1, "malformed line, expected `[warn]: text` after `^^^^^` part"); - auto message = codeLine[warn .. $].stripRight; - if (message.endsWith("+/", "*/")) - message = message[0 .. $ - 2].stripRight; - messages[lineNo - 1] = FoundWarning(message, start, end); - } - // Skip if no [warn] comment - else if (codeLine.indexOf("// [warn]:") != -1) - { - // Skip if there is no comment or code - immutable string codePart = codeLine.before("// "); - immutable string commentPart = codeLine.after("// "); - if (!codePart.length || !commentPart.length) - continue; - - // Get the message - messages[lineNo] = FoundWarning(commentPart); - } - } - - // Throw an assert error if any messages are not listed in the warnings - foreach (lineNo, message; messages) - { - // No warning - if (lineNo !in warnings) - { - immutable string errors = "Expected warning:\n%s\nFrom source code at (%s:?):\n%s".format(messages[lineNo], - lineNo, codeLines[lineNo - line]); - throw new AssertError(errors, file, lineNo); - } - // Different warning - else if (warnings[lineNo].msg != messages[lineNo].msg) - { - immutable string errors = "Expected warning:\n%s\nBut was:\n%s\nFrom source code at (%s:?):\n%s".format( - messages[lineNo], warnings[lineNo], lineNo, codeLines[lineNo - line]); - throw new AssertError(errors, file, lineNo); - } - - // specified column range - if ((message.startColumn || message.endColumn) - && warnings[lineNo] != message) - { - import std.algorithm : max; - import std.array : array; - import std.range : repeat; - import std.string : replace; - - const(char)[] expectedRange = ' '.repeat(max(0, cast(int)message.startColumn - 1)).array - ~ '^'.repeat(max(0, cast(int)(message.endColumn - message.startColumn))).array; - const(char)[] actualRange; - if (!warnings[lineNo].startColumn || warnings[lineNo].startColumn == warnings[lineNo].endColumn) - actualRange = "no column range defined!"; - else - actualRange = ' '.repeat(max(0, cast(int)warnings[lineNo].startColumn - 1)).array - ~ '^'.repeat(max(0, cast(int)(warnings[lineNo].endColumn - warnings[lineNo].startColumn))).array; - size_t paddingWidth = max(expectedRange.length, actualRange.length); - immutable string errors = "Wrong warning range: expected %s, but was %s\nFrom source code at (%s:?):\n%s\n%-*s <-- expected\n%-*s <-- actual".format( - [message.startColumn, message.endColumn], - [warnings[lineNo].startColumn, warnings[lineNo].endColumn], - lineNo, codeLines[lineNo - line].replace("\t", " "), - paddingWidth, expectedRange, - paddingWidth, actualRange); - throw new AssertError(errors, file, lineNo); - } - } - - // Throw an assert error if there were any warnings that were not expected - string[] unexpectedWarnings; - foreach (lineNo, warning; warnings) - { - // Unexpected warning - if (lineNo !in messages) - { - unexpectedWarnings ~= "%s\nFrom source code at (%s:?):\n%s".format(warning, - lineNo, codeLines[lineNo - line]); - } - } - if (unexpectedWarnings.length) - { - immutable string message = "Unexpected warnings:\n" ~ unexpectedWarnings.join("\n"); - throw new AssertError(message, file, line); - } -} - /// EOL inside this project, for tests private static immutable fileEol = q{ }; @@ -233,52 +62,29 @@ private static immutable fileEol = q{ * comment. Alternatively you can also just write `// fix` to apply the only * available suggestion. */ -void assertAutoFix(string before, string after, const StaticAnalysisConfig config, bool useDmd = false, - const AutoFixFormatting formattingConfig = AutoFixFormatting(AutoFixFormatting.BraceStyle.otbs, "\t", 4, fileEol), - string file = __FILE__, size_t line = __LINE__) +void assertAutoFix(string before, string after, const StaticAnalysisConfig config, + string file = __FILE__, size_t line = __LINE__) { - import dparse.lexer : StringCache, Token; - import dscanner.analysis.autofix : improveAutoFixWhitespace; - import dscanner.analysis.run : parseModule; import std.algorithm : canFind, findSplit, map, sort; import std.conv : to; import std.sumtype : match; import std.typecons : tuple, Tuple; - import std.file : exists, remove; - import std.path : dirName; - import std.stdio : File; - import dscanner.analysis.rundmd : analyzeDmd, parseDmdModule; - import dscanner.utils : getModuleName; + import dscanner.analysis.autofix : improveAutoFixWhitespace; MessageSet rawWarnings; - - if (useDmd) + auto testFileName = "test.d"; + File f = File(testFileName, "w"); + scope(exit) { - auto testFileName = "test.d"; - File f = File(testFileName, "w"); - scope(exit) - { - assert(exists(testFileName)); - remove(testFileName); - } - - f.rawWrite(before); - f.close(); - - auto dmdModule = parseDmdModule(file, before); - rawWarnings = analyzeDmd(testFileName, dmdModule, getModuleName(dmdModule.md), config); + assert(exists(testFileName)); + remove(testFileName); } - else - { - StringCache cache = StringCache(StringCache.defaultBucketCount); - RollbackAllocator r; - const(Token)[] tokens; - const(Module) m = parseModule(file, cast(ubyte[]) before, &r, defaultErrorFormat, cache, false, tokens); - ModuleCache moduleCache; + f.rawWrite(before); + f.close(); - rawWarnings = analyze("test", m, config, moduleCache, tokens, true, true, formattingConfig); - } + auto dmdModule = parseDmdModule(file, before); + rawWarnings = analyzeDmd(testFileName, dmdModule, getModuleName(dmdModule.md), config); string[] codeLines = before.splitLines(); Tuple!(Message, int)[] toApply; @@ -301,8 +107,7 @@ void assertAutoFix(string before, string after, const StaticAnalysisConfig confi immutable size_t rawLine = rawWarning.endLine; if (rawLine == 0) { - stderr.writefln("!!! Skipping warning because it is on line zero:\n%s", - rawWarning.message); + stderr.writefln("!!! Skipping warning because it is on line zero:\n%s", rawWarning.message); continue; } @@ -316,27 +121,22 @@ void assertAutoFix(string before, string after, const StaticAnalysisConfig confi assert(i >= 0, "can't use negative autofix indices"); if (i >= rawWarning.autofixes.length) throw new AssertError("autofix index out of range, diagnostic only has %s autofixes (%s)." - .format(rawWarning.autofixes.length, rawWarning.autofixes.map!"a.name"), - file, rawLine + line); + .format(rawWarning.autofixes.length, rawWarning.autofixes.map!"a.name"),file, rawLine + line); toApply ~= tuple(rawWarning, i); } else { if (rawWarning.autofixes.length != 1) throw new AssertError("diagnostic has %s autofixes (%s), but expected exactly one." - .format(rawWarning.autofixes.length, rawWarning.autofixes.map!"a.name"), - file, rawLine + line); + .format(rawWarning.autofixes.length, rawWarning.autofixes.map!"a.name"), file, rawLine + line); toApply ~= tuple(rawWarning, 0); } } } foreach (i, codeLine; codeLines) - { if (!applyLines.canFind(i) && codeLine.canFind("// fix")) - throw new AssertError("Missing expected warning for autofix on line %s" - .format(i + line), file, i + line); - } + throw new AssertError("Missing expected warning for autofix on line %s".format(i + line), file, i + line); AutoFix.CodeReplacement[] replacements; @@ -353,10 +153,7 @@ void assertAutoFix(string before, string after, const StaticAnalysisConfig confi string newCode = before; foreach_reverse (replacement; replacements) - { - newCode = newCode[0 .. replacement.range[0]] ~ replacement.newText - ~ newCode[replacement.range[1] .. $]; - } + newCode = newCode[0 .. replacement.range[0]] ~ replacement.newText ~ newCode[replacement.range[1] .. $]; if (newCode != after) { @@ -385,11 +182,6 @@ void assertAutoFix(string before, string after, const StaticAnalysisConfig confi void assertAnalyzerWarningsDMD(string code, const StaticAnalysisConfig config, bool semantic = false, string file = __FILE__, size_t line = __LINE__) { - import std.file : exists, remove; - import std.path : dirName; - import std.stdio : File; - import dscanner.analysis.rundmd : analyzeDmd, parseDmdModule; - import dscanner.utils : getModuleName; import dmd.globals : global; auto testFileName = "test.d"; @@ -483,6 +275,7 @@ void assertAnalyzerWarningsDMD(string code, const StaticAnalysisConfig config, b lineNo, codeLines[lineNo - line]); } } + if (unexpectedWarnings.length) { immutable string message = "Unexpected warnings:\n" ~ unexpectedWarnings.join("\n"); diff --git a/src/dscanner/analysis/lambda_return_check.d b/src/dscanner/analysis/lambda_return_check.d index 32f8468..cd5a6a6 100644 --- a/src/dscanner/analysis/lambda_return_check.d +++ b/src/dscanner/analysis/lambda_return_check.d @@ -139,7 +139,7 @@ unittest pragma(msg, typeof((a) { return a; })); // fix:0 pragma(msg, typeof((a) => () { return a; })); // fix:1 } - }c, sac, true); + }c, sac); stderr.writeln("Unittest for LambdaReturnCheck passed."); } diff --git a/src/dscanner/analysis/length_subtraction.d b/src/dscanner/analysis/length_subtraction.d index 6b35336..32c9229 100644 --- a/src/dscanner/analysis/length_subtraction.d +++ b/src/dscanner/analysis/length_subtraction.d @@ -68,7 +68,7 @@ unittest if (i < cast(ptrdiff_t) a.length - 1) // fix writeln("something"); } - }c, sac, true); + }c, sac); stderr.writeln("Unittest for IfElseSameCheck passed."); } diff --git a/src/dscanner/analysis/run.d b/src/dscanner/analysis/run.d index c919822..eba165c 100644 --- a/src/dscanner/analysis/run.d +++ b/src/dscanner/analysis/run.d @@ -22,14 +22,10 @@ import std.range; import std.stdio; import std.typecons : scoped; -import std.experimental.allocator : CAllocatorImpl; -import std.experimental.allocator.mallocator : Mallocator; -import std.experimental.allocator.building_blocks.region : Region; -import std.experimental.allocator.building_blocks.allocator_list : AllocatorList; - import dscanner.analysis.autofix : improveAutoFixWhitespace; import dscanner.analysis.config; import dscanner.analysis.base; +import dscanner.analysis.rundmd; import dscanner.analysis.style; import dscanner.analysis.enumarrayliteral; import dscanner.analysis.pokemon; @@ -79,23 +75,14 @@ import dscanner.analysis.redundant_storage_class; import dscanner.analysis.unused_result; import dscanner.analysis.cyclomatic_complexity; import dscanner.analysis.body_on_disabled_funcs; - -import dsymbol.string_interning : internString; -import dsymbol.scope_; -import dsymbol.semantic; -import dsymbol.conversion; -import dsymbol.conversion.first; -import dsymbol.conversion.second; -import dsymbol.modulecache : ModuleCache; - -import dscanner.utils; import dscanner.reports : DScannerJsonReporter, SonarQubeGenericIssueDataReporter; +import dscanner.utils; import dmd.astbase : ASTBase; -import dmd.parse : Parser; - -import dmd.frontend; import dmd.astcodegen; +import dmd.frontend; +import dmd.globals : global; +import dmd.parse : Parser; bool first = true; @@ -108,9 +95,6 @@ void doNothing(string, size_t, size_t, string, bool) { } -private alias ASTAllocator = CAllocatorImpl!( - AllocatorList!(n => Region!Mallocator(1024 * 128), Mallocator)); - immutable string defaultErrorFormat = "{filepath}({line}:{column})[{type}]: {message}"; string[string] errorFormatMap() @@ -261,9 +245,7 @@ void writeJSON(Message message) writeln(" {"); writeln(` "key": "`, message.key, `",`); if (message.checkName !is null) - { writeln(` "name": "`, message.checkName, `",`); - } writeln(` "fileName": "`, message.fileName.replace("\\", "\\\\").replace(`"`, `\"`), `",`); writeln(` "line": `, message.startLine, `,`); writeln(` "column": `, message.startColumn, `,`); @@ -299,42 +281,32 @@ void writeJSON(Message message) write(" }"); } -bool syntaxCheck(string[] fileNames, string errorFormat, ref StringCache stringCache, ref ModuleCache moduleCache) +bool syntaxCheck(string[] fileNames, string errorFormat) { StaticAnalysisConfig config = defaultStaticAnalysisConfig(); - return analyze(fileNames, config, errorFormat, stringCache, moduleCache, false); + return analyze(fileNames, config, errorFormat); } -void generateReport(string[] fileNames, const StaticAnalysisConfig config, - ref StringCache cache, ref ModuleCache moduleCache, string reportFile = "") +void generateReport(string[] fileNames, const StaticAnalysisConfig config, string reportFile = "") { auto reporter = new DScannerJsonReporter(); - - auto writeMessages = delegate void(string fileName, size_t line, size_t column, string message, bool isError){ - // TODO: proper index and column ranges - reporter.addMessage( - Message(Message.Diagnostic.from(fileName, [0, 0], line, [column, column], message), "dscanner.syntax"), - isError); - }; - first = true; - StatsCollector stats = new StatsCollector(BaseAnalyzerArguments.init); + auto statsCollector = new StatsCollector!ASTCodegen(); ulong lineOfCodeCount; + foreach (fileName; fileNames) { auto code = readFile(fileName); // Skip files that could not be read and continue with the rest if (code.length == 0) continue; - RollbackAllocator r; - const(Token)[] tokens; - const Module m = parseModule(fileName, code, &r, cache, tokens, writeMessages, &lineOfCodeCount, null, null); - stats.visit(m); - MessageSet messageSet = analyze(fileName, m, config, moduleCache, tokens, true); + auto dmdModule = parseDmdModule(fileName, cast(string) code); + dmdModule.accept(statsCollector); + MessageSet messageSet = analyzeDmd(fileName, dmdModule, getModuleName(dmdModule.md), config); reporter.addMessageSet(messageSet); } - string reportFileContent = reporter.getContent(stats, lineOfCodeCount); + string reportFileContent = reporter.getContent(statsCollector, lineOfCodeCount); if (reportFile == "") { writeln(reportFileContent); @@ -347,27 +319,18 @@ void generateReport(string[] fileNames, const StaticAnalysisConfig config, } void generateSonarQubeGenericIssueDataReport(string[] fileNames, const StaticAnalysisConfig config, - ref StringCache cache, ref ModuleCache moduleCache, string reportFile = "") + string reportFile = "") { auto reporter = new SonarQubeGenericIssueDataReporter(); - auto writeMessages = delegate void(string fileName, size_t line, size_t column, string message, bool isError){ - // TODO: proper index and column ranges - reporter.addMessage( - Message(Message.Diagnostic.from(fileName, [0, 0], line, [column, column], message), "dscanner.syntax"), - isError); - }; - foreach (fileName; fileNames) { auto code = readFile(fileName); // Skip files that could not be read and continue with the rest if (code.length == 0) continue; - RollbackAllocator r; - const(Token)[] tokens; - const Module m = parseModule(fileName, code, &r, cache, tokens, writeMessages, null, null, null); - MessageSet messageSet = analyze(fileName, m, config, moduleCache, tokens, true); + auto dmdModule = parseDmdModule(fileName, cast(string) code); + MessageSet messageSet = analyzeDmd(fileName, dmdModule, getModuleName(dmdModule.md), config); reporter.addMessageSet(messageSet); } @@ -388,45 +351,49 @@ void generateSonarQubeGenericIssueDataReport(string[] fileNames, const StaticAna * * Returns: true if there were errors or if there were warnings and `staticAnalyze` was true. */ -bool analyze(string[] fileNames, const StaticAnalysisConfig config, string errorFormat, - ref StringCache cache, ref ModuleCache moduleCache, bool staticAnalyze = true) +bool analyze(string[] fileNames, const StaticAnalysisConfig config, string errorFormat) { - import std.string : toStringz; - import dscanner.analysis.rundmd : parseDmdModule; - - import dscanner.analysis.rundmd : analyzeDmd; + import std.file : exists, remove; bool hasErrors; foreach (fileName; fileNames) { - auto code = readFile(fileName); + bool isStdin; + ubyte[] code; + + if (fileName == "stdin") + { + code = readStdin(); + fileName = "stdin.d"; + File f = File(fileName, "w"); + f.rawWrite(code); + f.close(); + isStdin = true; + } + else + { + code = readFile(fileName); + } // Skip files that could not be read and continue with the rest if (code.length == 0) continue; auto dmdModule = parseDmdModule(fileName, cast(string) code); - - RollbackAllocator r; - uint errorCount; - uint warningCount; - const(Token)[] tokens; - const Module m = parseModule(fileName, code, &r, errorFormat, cache, false, tokens, - null, &errorCount, &warningCount); - assert(m); - if (errorCount > 0 || (staticAnalyze && warningCount > 0)) + if (global.errors > 0 || global.warnings > 0) hasErrors = true; - MessageSet results = analyze(fileName, m, config, moduleCache, tokens, staticAnalyze); - MessageSet resultsDmd = analyzeDmd(fileName, dmdModule, getModuleName(dmdModule.md), config); - foreach (result; resultsDmd[]) - { - results.insert(result); - } + MessageSet results = analyzeDmd(fileName, dmdModule, getModuleName(dmdModule.md), config); + if (results is null) continue; + + hasErrors = !results.empty; foreach (result; results[]) - { - hasErrors = true; messageFunctionFormat(errorFormat, result, false, code); + + if (isStdin) + { + assert(exists(fileName)); + remove(fileName); } } return hasErrors; @@ -437,9 +404,7 @@ bool analyze(string[] fileNames, const StaticAnalysisConfig config, string error * * Returns: true if there were parse errors. */ -bool autofix(string[] fileNames, const StaticAnalysisConfig config, string errorFormat, - ref StringCache cache, ref ModuleCache moduleCache, bool autoApplySingle, - const AutoFixFormatting overrideFormattingConfig = AutoFixFormatting.invalid) +bool autofix(string[] fileNames, const StaticAnalysisConfig config, string errorFormat, bool autoApplySingle) { import std.format : format; @@ -450,16 +415,11 @@ bool autofix(string[] fileNames, const StaticAnalysisConfig config, string error // Skip files that could not be read and continue with the rest if (code.length == 0) continue; - RollbackAllocator r; - uint errorCount; - uint warningCount; - const(Token)[] tokens; - const Module m = parseModule(fileName, code, &r, errorFormat, cache, false, tokens, - null, &errorCount, &warningCount); - assert(m); - if (errorCount > 0) + auto dmdModule = parseDmdModule(fileName, cast(string) code); + if (global.errors > 0) hasErrors = true; - MessageSet results = analyze(fileName, m, config, moduleCache, tokens, true, true, overrideFormattingConfig); + + MessageSet results = analyzeDmd(fileName, dmdModule, getModuleName(dmdModule.md), config); if (results is null) continue; @@ -599,115 +559,6 @@ const(Module) parseModule(string fileName, ubyte[] code, RollbackAllocator* p, linesOfCode, errorCount, warningCount); } -/** -Checks whether a module is part of a user-specified include/exclude list. -The user can specify a comma-separated list of filters, everyone needs to start with -either a '+' (inclusion) or '-' (exclusion). -If no includes are specified, all modules are included. -*/ -bool shouldRun(check : BaseAnalyzer)(string moduleName, const ref StaticAnalysisConfig config) -{ - enum string a = check.name; - - if (mixin("config." ~ a) == Check.disabled) - return false; - - // By default, run the check - if (!moduleName.length) - return true; - - auto filters = mixin("config.filters." ~ a); - - // Check if there are filters are defined - // filters starting with a comma are invalid - if (filters.length == 0 || filters[0].length == 0) - return true; - - auto includers = filters.filter!(f => f[0] == '+').map!(f => f[1..$]); - auto excluders = filters.filter!(f => f[0] == '-').map!(f => f[1..$]); - - // exclusion has preference over inclusion - if (!excluders.empty && excluders.any!(s => moduleName.canFind(s))) - return false; - - if (!includers.empty) - return includers.any!(s => moduleName.canFind(s)); - - // by default: include all modules - return true; -} - -BaseAnalyzer[] getAnalyzersForModuleAndConfig(string fileName, - const(Token)[] tokens, const Module m, - const StaticAnalysisConfig analysisConfig, const Scope* moduleScope) -{ - BaseAnalyzer[] checks; - - string moduleName; - if (m !is null && m.moduleDeclaration !is null && - m.moduleDeclaration.moduleName !is null && - m.moduleDeclaration.moduleName.identifiers !is null) - moduleName = m.moduleDeclaration.moduleName.identifiers.map!(e => e.text).join("."); - - BaseAnalyzerArguments args = BaseAnalyzerArguments( - fileName, - tokens, - moduleScope - ); - - // Add those lines to suppress warnings about unused variables until cleanup is complete - bool ignoreVar = analysisConfig.if_constraints_indent == Check.skipTests; - bool ignoreVar2 = args.skipTests; - ignoreVar = ignoreVar || ignoreVar2; - - return checks; -} - -MessageSet analyze(string fileName, const Module m, const StaticAnalysisConfig analysisConfig, - ref ModuleCache moduleCache, const(Token)[] tokens, bool staticAnalyze = true, - bool resolveAutoFixes = false, - const AutoFixFormatting overrideFormattingConfig = AutoFixFormatting.invalid) -{ - import dsymbol.symbol : DSymbol; - import dscanner.analysis.autofix : resolveAutoFixFromCheck; - - if (!staticAnalyze) - return null; - - const(AutoFixFormatting) formattingConfig = - (resolveAutoFixes && overrideFormattingConfig is AutoFixFormatting.invalid) - ? analysisConfig.getAutoFixFormattingConfig() - : overrideFormattingConfig; - - scope first = new FirstPass(m, internString(fileName), &moduleCache, null); - first.run(); - - secondPass(first.rootSymbol, first.moduleScope, moduleCache); - auto moduleScope = first.moduleScope; - scope(exit) typeid(DSymbol).destroy(first.rootSymbol.acSymbol); - scope(exit) typeid(SemanticSymbol).destroy(first.rootSymbol); - scope(exit) typeid(Scope).destroy(first.moduleScope); - - GC.disable; - scope (exit) - GC.enable; - - MessageSet set = new MessageSet; - foreach (BaseAnalyzer check; getAnalyzersForModuleAndConfig(fileName, tokens, m, analysisConfig, moduleScope)) - { - check.visit(m); - foreach (message; check.messages) - { - if (resolveAutoFixes) - foreach (ref autofix; message.autofixes) - autofix.resolveAutoFixFromCheck(check, m, tokens, formattingConfig); - set.insert(message); - } - } - - return set; -} - version (unittest) { shared static this() diff --git a/src/dscanner/analysis/rundmd.d b/src/dscanner/analysis/rundmd.d index 8cbe4eb..8e5f005 100644 --- a/src/dscanner/analysis/rundmd.d +++ b/src/dscanner/analysis/rundmd.d @@ -97,6 +97,22 @@ private void setupDmd() MessageSet analyzeDmd(string fileName, ASTCodegen.Module m, const char[] moduleName, const StaticAnalysisConfig config) { MessageSet set = new MessageSet; + auto visitors = getDmdAnalyzersForModuleAndConfig(fileName, config, moduleName); + + foreach (visitor; visitors) + { + m.accept(visitor); + + foreach (message; visitor.messages) + set.insert(message); + } + + return set; +} + +BaseAnalyzerDmd[] getDmdAnalyzersForModuleAndConfig(string fileName, const StaticAnalysisConfig config, + const char[] moduleName) +{ BaseAnalyzerDmd[] visitors; if (moduleName.shouldRunDmd!(ObjectConstCheck!ASTCodegen)(config)) @@ -322,7 +338,7 @@ MessageSet analyzeDmd(string fileName, ASTCodegen.Module m, const char[] moduleN fileName, config.vcall_in_ctor == Check.skipTests && !ut ); - + if (moduleName.shouldRunDmd!AllManCheck(config)) visitors ~= new AllManCheck( fileName, @@ -353,15 +369,7 @@ MessageSet analyzeDmd(string fileName, ASTCodegen.Module m, const char[] moduleN config.function_attribute_check == Check.skipTests && !ut ); - foreach (visitor; visitors) - { - m.accept(visitor); - - foreach (message; visitor.messages) - set.insert(message); - } - - return set; + return visitors; } /** diff --git a/src/dscanner/analysis/static_if_else.d b/src/dscanner/analysis/static_if_else.d index 7ce1aec..6be6180 100644 --- a/src/dscanner/analysis/static_if_else.d +++ b/src/dscanner/analysis/static_if_else.d @@ -120,7 +120,8 @@ extern (C++) class StaticIfElse(AST) : BaseAnalyzerDmd .map!(t => t.loc.fileOffset + 1) .array; - AutoFix autofix2 = AutoFix.insertionAt(ifStmt.endloc.fileOffset, braceEnd); + AutoFix autofix2 = + AutoFix.insertionAt(ifStmt.endloc.fileOffset, braceEnd, "Wrap '{}' block around 'if'"); foreach (fileOffset; fileOffsets) autofix2 = autofix2.concat(AutoFix.insertionAt(fileOffset, "\t")); autofix2 = autofix2.concat(AutoFix.insertionAt(ifStmt.loc.fileOffset, braceStart)); @@ -139,13 +140,12 @@ extern (C++) class StaticIfElse(AST) : BaseAnalyzerDmd autofix2 = autofix2.concat(AutoFix.insertionAt(ifStmt.ifbody.loc.fileOffset, "\t")); } - + ulong[2] index = [cast(ulong) s.elsebody.loc.fileOffset - 5, cast(ulong) ifStmt.loc.fileOffset + 2]; + ulong[2] lines = [cast(ulong) s.elsebody.loc.linnum, cast(ulong) ifStmt.loc.linnum]; + ulong[2] columns = [cast(ulong) s.elsebody.loc.charnum, cast(ulong) ifStmt.loc.charnum + 2]; addErrorMessage( - cast(ulong) ifStmt.loc.linnum, cast(ulong) s.elsebody.loc.charnum, KEY, MESSAGE, - [ - AutoFix.insertionAt(ifStmt.loc.fileOffset, "static "), - autofix2 - ] + index, lines, columns, KEY, MESSAGE, + [AutoFix.insertionAt(ifStmt.loc.fileOffset, "static "), autofix2] ); } @@ -227,7 +227,7 @@ unittest } } } - }c, sac, true); + }c, sac); stderr.writeln("Unittest for StaticIfElse passed."); } diff --git a/src/dscanner/analysis/stats_collector.d b/src/dscanner/analysis/stats_collector.d index 4d69730..5ba7208 100644 --- a/src/dscanner/analysis/stats_collector.d +++ b/src/dscanner/analysis/stats_collector.d @@ -5,62 +5,154 @@ module dscanner.analysis.stats_collector; -import dparse.ast; -import dparse.lexer; import dscanner.analysis.base; -final class StatsCollector : BaseAnalyzer +extern (C++) class StatsCollector(AST) : BaseAnalyzerDmd { - alias visit = ASTVisitor.visit; + alias visit = BaseAnalyzerDmd.visit; + mixin AnalyzerInfo!"stats_collector"; - this(BaseAnalyzerArguments args) + public uint interfaceCount; + public uint classCount; + public uint functionCount; + public uint templateCount; + public uint structCount; + public uint statementCount; + // TODO: Count lines of code + public uint lineOfCodeCount; + // TODO: Count undocumented public symbols + public uint undocumentedPublicSymbols; + + extern (D) this(string fileName = "", bool skipTests = false) { - args.skipTests = false; // old behavior compatibility - super(args); + super(fileName, skipTests); } - override void visit(const Statement statement) - { - statementCount++; - statement.accept(this); - } - - override void visit(const ClassDeclaration classDeclaration) - { - classCount++; - classDeclaration.accept(this); - } - - override void visit(const InterfaceDeclaration interfaceDeclaration) + override void visit(AST.InterfaceDeclaration interfaceDecl) { interfaceCount++; - interfaceDeclaration.accept(this); + super.visit(interfaceDecl); } - override void visit(const FunctionDeclaration functionDeclaration) + override void visit(AST.ClassDeclaration classDecl) + { + classCount++; + super.visit(classDecl); + } + + override void visit(AST.FuncDeclaration funcDecl) { functionCount++; - functionDeclaration.accept(this); + super.visit(funcDecl); } - override void visit(const StructDeclaration structDeclaration) - { - structCount++; - structDeclaration.accept(this); - } - - override void visit(const TemplateDeclaration templateDeclaration) + override void visit(AST.TemplateDeclaration templateDecl) { templateCount++; - templateDeclaration.accept(this); + super.visit(templateDecl); } - uint interfaceCount; - uint classCount; - uint functionCount; - uint templateCount; - uint structCount; - uint statementCount; - uint lineOfCodeCount; - uint undocumentedPublicSymbols; + override void visit(AST.StructDeclaration structDecl) + { + structCount++; + super.visit(structDecl); + } + + mixin VisitStatement!(AST.ErrorStatement); + mixin VisitStatement!(AST.PeelStatement); + mixin VisitStatement!(AST.ScopeStatement); + mixin VisitStatement!(AST.ExpStatement); + mixin VisitStatement!(AST.ReturnStatement); + mixin VisitStatement!(AST.IfStatement); + mixin VisitStatement!(AST.CaseStatement); + mixin VisitStatement!(AST.DefaultStatement); + mixin VisitStatement!(AST.LabelStatement); + mixin VisitStatement!(AST.GotoStatement); + mixin VisitStatement!(AST.GotoDefaultStatement); + mixin VisitStatement!(AST.GotoCaseStatement); + mixin VisitStatement!(AST.BreakStatement); + mixin VisitStatement!(AST.DtorExpStatement); + mixin VisitStatement!(AST.MixinStatement); + mixin VisitStatement!(AST.ForwardingStatement); + mixin VisitStatement!(AST.DoStatement); + mixin VisitStatement!(AST.WhileStatement); + mixin VisitStatement!(AST.ForStatement); + mixin VisitStatement!(AST.ForeachStatement); + mixin VisitStatement!(AST.SwitchStatement); + mixin VisitStatement!(AST.ContinueStatement); + mixin VisitStatement!(AST.WithStatement); + mixin VisitStatement!(AST.TryCatchStatement); + mixin VisitStatement!(AST.ThrowStatement); + mixin VisitStatement!(AST.DebugStatement); + mixin VisitStatement!(AST.TryFinallyStatement); + mixin VisitStatement!(AST.ScopeGuardStatement); + mixin VisitStatement!(AST.SwitchErrorStatement); + mixin VisitStatement!(AST.UnrolledLoopStatement); + mixin VisitStatement!(AST.ForeachRangeStatement); + mixin VisitStatement!(AST.CompoundDeclarationStatement); + mixin VisitStatement!(AST.CompoundAsmStatement); + mixin VisitStatement!(AST.StaticAssertStatement); + mixin VisitStatement!(AST.CaseRangeStatement); + mixin VisitStatement!(AST.SynchronizedStatement); + mixin VisitStatement!(AST.AsmStatement); + mixin VisitStatement!(AST.InlineAsmStatement); + mixin VisitStatement!(AST.GccAsmStatement); + mixin VisitStatement!(AST.ImportStatement); + + private template VisitStatement(NodeType) + { + override void visit(NodeType node) + { + statementCount++; + super.visit(node); + } + } +} + +unittest +{ + import std.file : exists, remove; + import std.path : dirName; + import std.stdio : File, stderr; + import dscanner.analysis.rundmd : parseDmdModule; + import dmd.astcodegen : ASTCodegen; + + string code = q{ + interface I {} + class C {} + void f() {} + template T() {} + struct S {} + + void funcWithStatements() + { + int a = 1; + if (a == 1) + a = 2; + a++; + } + }c; + + auto testFileName = "test.d"; + File f = File(testFileName, "w"); + scope(exit) + { + assert(exists(testFileName)); + remove(testFileName); + } + + f.rawWrite(code); + f.close(); + auto dmdModule = parseDmdModule(testFileName, code); + auto collector = new StatsCollector!ASTCodegen(); + dmdModule.accept(collector); + + assert(collector.interfaceCount == 1); + assert(collector.classCount == 1); + assert(collector.functionCount == 2); + assert(collector.templateCount == 1); + assert(collector.structCount == 1); + assert(collector.statementCount == 4); + + stderr.writeln("Unittest for StatsCollector passed."); } diff --git a/src/dscanner/analysis/style.d b/src/dscanner/analysis/style.d index 401649d..42347d6 100644 --- a/src/dscanner/analysis/style.d +++ b/src/dscanner/analysis/style.d @@ -7,6 +7,7 @@ module dscanner.analysis.style; import dscanner.analysis.base; import dmd.astenums : LINK; +import dmd.location : Loc; import std.conv : to; import std.format : format; import std.regex; @@ -39,19 +40,17 @@ extern (C++) class StyleChecker(AST) : BaseAnalyzerDmd return; auto moduleDecl = *moduleNode.md; - auto lineNum = cast(ulong) moduleDecl.loc.linnum; - auto charNum = cast(ulong) moduleDecl.loc.charnum; - auto moduleName = moduleDecl.id.toString(); + auto moduleName = cast(string) moduleDecl.id.toString(); if (moduleName.matchFirst(moduleNameRegex).length == 0) - addErrorMessage(lineNum, charNum, KEY, MSG.format("Module/package", moduleName)); + addError(moduleDecl.loc, "Module/package", moduleName); foreach (pkg; moduleDecl.packages) { auto pkgName = pkg.toString(); if (pkgName.matchFirst(moduleNameRegex).length == 0) - addErrorMessage(lineNum, charNum, KEY, MSG.format("Module/package", pkgName)); + addError(moduleDecl.loc, "Module/package", moduleName); } } @@ -80,15 +79,10 @@ extern (C++) class StyleChecker(AST) : BaseAnalyzerDmd if (varDeclaration.storage_class & STC.manifest || varDeclaration.ident is null) return; - auto varName = varDeclaration.ident.toString(); + auto varName = cast(string) varDeclaration.ident.toString(); if (varName.matchFirst(varFunNameRegex).length == 0) - { - auto msg = MSG.format("Variable", varName); - auto lineNum = cast(ulong) varDeclaration.loc.linnum; - auto charNum = cast(ulong) varDeclaration.loc.charnum; - addErrorMessage(lineNum, charNum, KEY, msg); - } + addError(varDeclaration.loc, "Variable", varName); } mixin VisitNode!(AST.ClassDeclaration, "Class", aggregateNameRegex); @@ -108,17 +102,23 @@ extern (C++) class StyleChecker(AST) : BaseAnalyzerDmd if (node.ident is null) return; - auto nodeSymbolName = node.ident.toString(); + auto nodeSymbolName = cast(string) node.ident.toString(); if (nodeSymbolName.matchFirst(regex).length == 0) - { - auto msg = MSG.format(nodeName, nodeSymbolName); - auto lineNum = cast(ulong) node.loc.linnum; - auto charNum = cast(ulong) node.loc.charnum; - addErrorMessage(lineNum, charNum, KEY, msg); - } + addError(node.loc, nodeName, nodeSymbolName); } } + + private extern (D) void addError(Loc loc, string nodeType, string nodeName) + { + auto fileOffset = cast(ulong) loc.fileOffset; + auto lineNum = cast(ulong) loc.linnum; + auto charNum = cast(ulong) loc.charnum; + ulong[2] index = [fileOffset, fileOffset + nodeName.length]; + ulong[2] lines = [lineNum, lineNum]; + ulong[2] columns = [charNum, charNum + nodeName.length]; + addErrorMessage(index, lines, columns, KEY, MSG.format(nodeType, nodeName)); + } } unittest diff --git a/src/dscanner/main.d b/src/dscanner/main.d index d6a74fa..70e9090 100644 --- a/src/dscanner/main.d +++ b/src/dscanner/main.d @@ -325,11 +325,11 @@ else if (autofix) { - return .autofix(expandedArgs, config, errorFormat, cache, moduleCache, applySingleFixes) ? 1 : 0; + return .autofix(expandedArgs, config, errorFormat, applySingleFixes) ? 1 : 0; } else if (resolveMessage.length) { - listAutofixes(config, resolveMessage, usingStdin, usingStdin ? "stdin" : args[1], &cache, moduleCache); + listAutofixes(config, resolveMessage, usingStdin, usingStdin ? "stdin" : args[1]); return 0; } else if (report) @@ -341,19 +341,19 @@ else goto case; case "": case "dscanner": - generateReport(expandedArgs, config, cache, moduleCache, reportFile); + generateReport(expandedArgs, config, reportFile); break; case "sonarQubeGenericIssueData": - generateSonarQubeGenericIssueDataReport(expandedArgs, config, cache, moduleCache, reportFile); + generateSonarQubeGenericIssueDataReport(expandedArgs, config, reportFile); break; } } else - return analyze(expandedArgs, config, errorFormat, cache, moduleCache, true) ? 1 : 0; + return analyze(expandedArgs, config, errorFormat) ? 1 : 0; } else if (syntaxCheck) { - return .syntaxCheck(usingStdin ? ["stdin"] : expandedArgs, errorFormat, cache, moduleCache) ? 1 : 0; + return .syntaxCheck(usingStdin ? ["stdin"] : expandedArgs, errorFormat) ? 1 : 0; } else { diff --git a/src/dscanner/reports.d b/src/dscanner/reports.d index c35d9b0..0d5cc46 100644 --- a/src/dscanner/reports.d +++ b/src/dscanner/reports.d @@ -37,7 +37,7 @@ class DScannerJsonReporter _issues ~= toIssue(message, isError); } - string getContent(StatsCollector stats, ulong lineOfCodeCount) + string getContent(AST)(StatsCollector!AST stats, ulong lineOfCodeCount) { JSONValue result = [ "issues" : JSONValue(_issues.data.map!(e => toJson(e)).array), diff --git a/tests/it.sh b/tests/it.sh index b4990d1..5df4a57 100755 --- a/tests/it.sh +++ b/tests/it.sh @@ -41,7 +41,12 @@ cd "$DSCANNER_DIR/tests" # IDE APIs # -------- # checking that reporting format stays consistent or only gets extended -diff <(../bin/dscanner --report it/autofix_ide/source_autofix.d | jq -S .) <(jq -S . it/autofix_ide/source_autofix.report.json) +if [[ $1 == "Windows" ]]; then + diff <(../bin/dscanner --report it/autofix_ide/source_autofix.d | jq -S .) <(jq -S . it/autofix_ide/source_autofix_windows.report.json) +else + diff <(../bin/dscanner --report it/autofix_ide/source_autofix.d | jq -S .) <(jq -S . it/autofix_ide/source_autofix.report.json) +fi + diff <(../bin/dscanner --resolveMessage b16 it/autofix_ide/source_autofix.d | jq -S .) <(jq -S . it/autofix_ide/source_autofix.autofix.json) # CLI tests diff --git a/tests/it/autofix_ide/source_autofix.autofix.json b/tests/it/autofix_ide/source_autofix.autofix.json index fa4c066..b66071d 100644 --- a/tests/it/autofix_ide/source_autofix.autofix.json +++ b/tests/it/autofix_ide/source_autofix.autofix.json @@ -1,36 +1,36 @@ [ { - "name": "Mark function `const`", + "name": "Insert `const`", "replacements": [ { - "newText": " const", + "newText": "const ", "range": [ - 24, - 24 + 25, + 25 ] } ] }, { - "name": "Mark function `inout`", + "name": "Insert `inout`", "replacements": [ { - "newText": " inout", + "newText": "inout ", "range": [ - 24, - 24 + 25, + 25 ] } ] }, { - "name": "Mark function `immutable`", + "name": "Insert `immutable`", "replacements": [ { - "newText": " immutable", + "newText": "immutable ", "range": [ - 24, - 24 + 25, + 25 ] } ] diff --git a/tests/it/autofix_ide/source_autofix.report.json b/tests/it/autofix_ide/source_autofix.report.json index 909fa23..e639ce9 100644 --- a/tests/it/autofix_ide/source_autofix.report.json +++ b/tests/it/autofix_ide/source_autofix.report.json @@ -18,37 +18,37 @@ "type": "warn", "autofixes": [ { - "name": "Mark function `const`", + "name": "Insert `const`", "replacements": [ { - "newText": " const", + "newText": "const ", "range": [ - 24, - 24 + 25, + 25 ] } ] }, { - "name": "Mark function `inout`", + "name": "Insert `inout`", "replacements": [ { - "newText": " inout", + "newText": "inout ", "range": [ - 24, - 24 + 25, + 25 ] } ] }, { - "name": "Mark function `immutable`", + "name": "Insert `immutable`", "replacements": [ { - "newText": " immutable", + "newText": "immutable ", "range": [ - 24, - 24 + 25, + 25 ] } ] @@ -71,10 +71,53 @@ }, { "name": "Wrap '{}' block around 'if'", - "replacements": "resolvable" + "replacements": [ + { + "newText": " {\n\t\t\t", + "range": [ + 69, + 69 + ] + }, + { + "newText": "\t", + "range": [ + 76, + 76 + ] + }, + { + "newText": "\t", + "range": [ + 80, + 80 + ] + }, + { + "newText": "\t", + "range": [ + 82, + 82 + ] + }, + { + "newText": "\t", + "range": [ + 84, + 84 + ] + }, + { + "newText": "}\n\t", + "range": [ + 85, + 85 + ] + } + ] } ], - "column": 3, + "column": 8, "endColumn": 10, "endIndex": 71, "endLine": 8, @@ -88,8 +131,8 @@ "type": "warn" } ], - "lineOfCodeCount": 3, - "statementCount": 4, + "lineOfCodeCount": 0, + "statementCount": 2, "structCount": 1, "templateCount": 0, "undocumentedPublicSymbols": 0 diff --git a/tests/it/autofix_ide/source_autofix_windows.report.json b/tests/it/autofix_ide/source_autofix_windows.report.json new file mode 100644 index 0000000..3132db7 --- /dev/null +++ b/tests/it/autofix_ide/source_autofix_windows.report.json @@ -0,0 +1,139 @@ +{ + "classCount": 0, + "functionCount": 1, + "interfaceCount": 0, + "issues": [ + { + "column": 6, + "endColumn": 12, + "endIndex": 22, + "endLine": 3, + "fileName": "it/autofix_ide/source_autofix.d", + "index": 16, + "key": "dscanner.confusing.function_attributes", + "line": 3, + "message": "Zero-parameter '@property' function should be marked 'const', 'inout', or 'immutable'.", + "name": "function_attribute_check", + "supplemental": [], + "type": "warn", + "autofixes": [ + { + "name": "Insert `const`", + "replacements": [ + { + "newText": "const ", + "range": [ + 25, + 25 + ] + } + ] + }, + { + "name": "Insert `inout`", + "replacements": [ + { + "newText": "inout ", + "range": [ + 25, + 25 + ] + } + ] + }, + { + "name": "Insert `immutable`", + "replacements": [ + { + "newText": "immutable ", + "range": [ + 25, + 25 + ] + } + ] + } + ] + }, + { + "autofixes": [ + { + "name": "Insert `static`", + "replacements": [ + { + "newText": "static ", + "range": [ + 69, + 69 + ] + } + ] + }, + { + "name": "Wrap '{}' block around 'if'", + "replacements": [ + { + "newText": " {\r\n\t\t\t", + "range": [ + 69, + 69 + ] + }, + { + "newText": "\t", + "range": [ + 76, + 76 + ] + }, + { + "newText": "\t", + "range": [ + 80, + 80 + ] + }, + { + "newText": "\t", + "range": [ + 82, + 82 + ] + }, + { + "newText": "\t", + "range": [ + 84, + 84 + ] + }, + { + "newText": "}\r\n\t", + "range": [ + 85, + 85 + ] + } + ] + } + ], + "column": 8, + "endColumn": 10, + "endIndex": 71, + "endLine": 8, + "fileName": "it/autofix_ide/source_autofix.d", + "index": 64, + "key": "dscanner.suspicious.static_if_else", + "line": 8, + "message": "Mismatched static if. Use 'else static if' here.", + "name": "static_if_else_check", + "supplemental": [], + "type": "warn" + } + ], + "lineOfCodeCount": 0, + "statementCount": 2, + "structCount": 1, + "templateCount": 0, + "undocumentedPublicSymbols": 0 +} \ No newline at end of file