From f6bfa57898d2ff66bb52476c9119f13057e7a326 Mon Sep 17 00:00:00 2001 From: Vladiwostok <55026261+Vladiwostok@users.noreply.github.com> Date: Fri, 8 Nov 2024 09:35:10 +0200 Subject: [PATCH] Replace libdparse with DMD in IfConstraintsIndentCheck (#128) * Replace libdparse with DMD in IfConstraintsIndentCheck * Fix evil segfault bug * Remove Issue#829 unit test * Properly detect issue --- src/dscanner/analysis/if_constraints_indent.d | 267 ++++++++---------- src/dscanner/analysis/run.d | 4 - src/dscanner/analysis/rundmd.d | 7 + 3 files changed, 120 insertions(+), 158 deletions(-) diff --git a/src/dscanner/analysis/if_constraints_indent.d b/src/dscanner/analysis/if_constraints_indent.d index d1b31441..831988bb 100644 --- a/src/dscanner/analysis/if_constraints_indent.d +++ b/src/dscanner/analysis/if_constraints_indent.d @@ -4,194 +4,132 @@ module dscanner.analysis.if_constraints_indent; -import dparse.lexer; -import dparse.ast; import dscanner.analysis.base; -import dsymbol.scope_ : Scope; - -import std.algorithm.iteration : filter; -import std.range; +import dmd.tokens : Token, TOK; /** Checks whether all if constraints have the same indention as their declaration. */ -final class IfConstraintsIndentCheck : BaseAnalyzer +extern (C++) class IfConstraintsIndentCheck(AST) : BaseAnalyzerDmd { + alias visit = BaseAnalyzerDmd.visit; mixin AnalyzerInfo!"if_constraints_indent"; - /// - this(BaseAnalyzerArguments args) - { - super(args); + private enum string KEY = "dscanner.style.if_constraints_indent"; + private enum string MSG = "If constraints should have the same indentation as the function"; - // convert tokens to a list of token starting positions per line + private Token[] tokens; - // libdparse columns start at 1 - foreach (t; tokens) - { - // pad empty positions if we skip empty token-less lines - // t.line (unsigned) may be 0 if the token is uninitialized/broken, so don't subtract from it - // equivalent to: firstSymbolAtLine.length < t.line - 1 - while (firstSymbolAtLine.length + 1 < t.line) - firstSymbolAtLine ~= Pos(1, t.index); - - // insert a new line with positions if new line is reached - // (previous while pads skipped lines) - if (firstSymbolAtLine.length < t.line) - firstSymbolAtLine ~= Pos(t.column, t.index, t.type == tok!"if"); - } - } - - override void visit(const FunctionDeclaration decl) + extern (D) this(string fileName, bool skipTests = false) { - if (decl.constraint !is null) - checkConstraintSpace(decl.constraint, decl.name); + super(fileName, skipTests); + lexFile(); } - override void visit(const InterfaceDeclaration decl) + private void lexFile() { - if (decl.constraint !is null) - checkConstraintSpace(decl.constraint, decl.name); - } + import dscanner.utils : readFile; + import dmd.errorsink : ErrorSinkNull; + import dmd.globals : global; + import dmd.lexer : Lexer; + auto bytes = readFile(fileName) ~ '\0'; - override void visit(const ClassDeclaration decl) - { - if (decl.constraint !is null) - checkConstraintSpace(decl.constraint, decl.name); - } + __gshared ErrorSinkNull errorSinkNull; + if (!errorSinkNull) + errorSinkNull = new ErrorSinkNull; - override void visit(const TemplateDeclaration decl) - { - if (decl.constraint !is null) - checkConstraintSpace(decl.constraint, decl.name); - } - - override void visit(const UnionDeclaration decl) - { - if (decl.constraint !is null) - checkConstraintSpace(decl.constraint, decl.name); - } + scope lexer = new Lexer(null, cast(char*) bytes, 0, bytes.length, 0, 0, errorSinkNull, &global.compileEnv); - override void visit(const StructDeclaration decl) - { - if (decl.constraint !is null) - checkConstraintSpace(decl.constraint, decl.name); - } - - override void visit(const Constructor decl) - { - if (decl.constraint !is null) - checkConstraintSpace(decl.constraint, decl.line); - } - - alias visit = ASTVisitor.visit; - -private: - - enum string KEY = "dscanner.style.if_constraints_indent"; - enum string MESSAGE = "If constraints should have the same indentation as the function"; - - Pos[] firstSymbolAtLine; - static struct Pos - { - size_t column; - size_t index; - bool isIf; - } - - /** - Check indentation of constraints - */ - void checkConstraintSpace(const Constraint constraint, const Token token) - { - checkConstraintSpace(constraint, token.line); + do + { + lexer.nextToken(); + tokens ~= lexer.token; + } + while (lexer.token.value != TOK.endOfFile); } - void checkConstraintSpace(const Constraint constraint, size_t line) + override void visit(AST.TemplateDeclaration templateDecl) { - import std.algorithm : min; - - // dscanner lines start at 1 - auto pDecl = firstSymbolAtLine[line - 1]; - - // search for constraint if (might not be on the same line as the expression) - auto r = firstSymbolAtLine[line .. constraint.expression.line].retro.filter!(s => s.isIf); - - auto if_ = constraint.tokens.findTokenForDisplay(tok!"if")[0]; - - // no hit = constraint is on the same line - if (r.empty) - addErrorMessage(if_, KEY, MESSAGE); - else if (pDecl.column != r.front.column) - addErrorMessage([min(if_.index, pDecl.index), if_.index + 2], if_.line, [min(if_.column, pDecl.column), if_.column + 2], KEY, MESSAGE); + import std.array : array; + import std.algorithm : filter; + import std.range : front, retro; + + super.visit(templateDecl); + + if (templateDecl.constraint is null || templateDecl.members is null) + return; + + auto firstTemplateToken = tokens.filter!(t => t.loc.linnum == templateDecl.loc.linnum) + .filter!(t => t.value != TOK.whitespace) + .front; + uint templateLine = firstTemplateToken.loc.linnum; + uint templateCol = firstTemplateToken.loc.charnum; + + auto constraintToken = tokens.filter!(t => t.loc.fileOffset <= templateDecl.constraint.loc.fileOffset) + .array() + .retro() + .filter!(t => t.value == TOK.if_) + .front; + uint constraintLine = constraintToken.loc.linnum; + uint constraintCol = constraintToken.loc.charnum; + + if (templateLine == constraintLine || templateCol != constraintCol) + addErrorMessage(cast(ulong) constraintLine, cast(ulong) constraintCol, KEY, MSG); } } unittest { import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig; - import dscanner.analysis.helpers : assertAnalyzerWarnings; + import dscanner.analysis.helpers : assertAnalyzerWarningsDMD; import std.format : format; import std.stdio : stderr; StaticAnalysisConfig sac = disabledConfig(); sac.if_constraints_indent = Check.enabled; + enum MSG = "If constraints should have the same indentation as the function"; - assertAnalyzerWarnings(q{ + assertAnalyzerWarningsDMD(q{ void foo(R)(R r) if (R == null) {} -// note: since we are using tabs, the ^ look a bit off here. Use 1-wide tab stops to view tests. void foo(R)(R r) - if (R == null) /+ -^^^ [warn]: %s +/ + if (R == null) // [warn]: %s {} - }c.format( - IfConstraintsIndentCheck.MESSAGE, - ), sac); + }c.format(MSG), sac); - assertAnalyzerWarnings(q{ + assertAnalyzerWarningsDMD(q{ void foo(R)(R r) if (R == null) {} void foo(R)(R r) -if (R == null) /+ -^^ [warn]: %s +/ +if (R == null) // [warn]: %s {} void foo(R)(R r) - if (R == null) /+ - ^^^ [warn]: %s +/ + if (R == null) // [warn]: %s {} - }c.format( - IfConstraintsIndentCheck.MESSAGE, - IfConstraintsIndentCheck.MESSAGE, - ), sac); + }c.format(MSG, MSG), sac); - assertAnalyzerWarnings(q{ + assertAnalyzerWarningsDMD(q{ struct Foo(R) if (R == null) {} struct Foo(R) -if (R == null) /+ -^^ [warn]: %s +/ +if (R == null) // [warn]: %s {} struct Foo(R) - if (R == null) /+ - ^^^ [warn]: %s +/ + if (R == null) // [warn]: %s {} - }c.format( - IfConstraintsIndentCheck.MESSAGE, - IfConstraintsIndentCheck.MESSAGE, - ), sac); + }c.format(MSG, MSG), sac); // test example from Phobos - assertAnalyzerWarnings(q{ + assertAnalyzerWarningsDMD(q{ Num abs(Num)(Num x) @safe pure nothrow if (is(typeof(Num.init >= 0)) && is(typeof(-Num.init)) && !(is(Num* : const(ifloat*)) || is(Num* : const(idouble*)) @@ -205,7 +143,7 @@ if (is(typeof(Num.init >= 0)) && is(typeof(-Num.init)) && }, sac); // weird constraint formatting - assertAnalyzerWarnings(q{ + assertAnalyzerWarningsDMD(q{ struct Foo(R) if (R == null) @@ -217,8 +155,7 @@ if (is(typeof(Num.init >= 0)) && is(typeof(-Num.init)) && {} struct Foo(R) -if /+ -^^ [warn]: %s +/ +if // [warn]: %s (R == null) {} @@ -234,39 +171,61 @@ if /+ {} struct Foo(R) - if ( /+ - ^^^ [warn]: %s +/ + if ( // [warn]: %s R == null ) {} - }c.format( - IfConstraintsIndentCheck.MESSAGE, - IfConstraintsIndentCheck.MESSAGE, - ), sac); + }c.format(MSG, MSG), sac); // constraint on the same line - assertAnalyzerWarnings(q{ - struct CRC(uint N, ulong P) if (N == 32 || N == 64) /+ - ^^ [warn]: %s +/ + assertAnalyzerWarningsDMD(q{ + struct CRC(uint N, ulong P) if (N == 32 || N == 64) // [warn]: %s {} - }c.format( - IfConstraintsIndentCheck.MESSAGE, - ), sac); + }c.format(MSG), sac); - stderr.writeln("Unittest for IfConstraintsIndentCheck passed."); + assertAnalyzerWarningsDMD(q{ +private template sharedToString(alias field) +if (is(typeof(field) == shared)) +{ + static immutable sharedToString = typeof(field).stringof; } + }c, sac); -@("issue #829") -unittest + assertAnalyzerWarningsDMD(q{ +private union EndianSwapper(T) +if (canSwapEndianness!T) { - import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig; - import dscanner.analysis.helpers : assertAnalyzerWarnings; - import std.format : format; - import std.stdio : stderr; + T value; +} + }c, sac); - StaticAnalysisConfig sac = disabledConfig(); - sac.if_constraints_indent = Check.enabled; + assertAnalyzerWarningsDMD(q{ +void test(alias matchFn)() +{ + auto baz(Cap)(Cap m) + if (is(Cap == Captures!(Cap.String))) + { + return toUpper(m.hit); + } +} + }c, sac); - assertAnalyzerWarnings(`void foo() { - f; -}`, sac); + assertAnalyzerWarningsDMD(q{ +ElementType!(A) pop (A) (ref A a) +if (isDynamicArray!(A) && !isNarrowString!(A) && isMutable!(A) && !is(A == void[])) +{ + auto e = a.back; + a.popBack(); + return e; +} + }c, sac); + + assertAnalyzerWarningsDMD(q{ + template HMAC(H) + if (isDigest!H && hasBlockSize!H) + { + alias HMAC = HMAC!(H, H.blockSize); + } + }, sac); + + stderr.writeln("Unittest for IfConstraintsIndentCheck passed."); } diff --git a/src/dscanner/analysis/run.d b/src/dscanner/analysis/run.d index fba4c116..8e68830f 100644 --- a/src/dscanner/analysis/run.d +++ b/src/dscanner/analysis/run.d @@ -663,10 +663,6 @@ BaseAnalyzer[] getAnalyzersForModuleAndConfig(string fileName, checks ~= new UndocumentedDeclarationCheck(args.setSkipTests( analysisConfig.undocumented_declaration_check == Check.skipTests && !ut)); - if (moduleName.shouldRun!IfConstraintsIndentCheck(analysisConfig)) - checks ~= new IfConstraintsIndentCheck(args.setSkipTests( - analysisConfig.if_constraints_indent == Check.skipTests && !ut)); - return checks; } diff --git a/src/dscanner/analysis/rundmd.d b/src/dscanner/analysis/rundmd.d index 0cf8e17a..594dec63 100644 --- a/src/dscanner/analysis/rundmd.d +++ b/src/dscanner/analysis/rundmd.d @@ -25,6 +25,7 @@ import dscanner.analysis.enumarrayliteral : EnumArrayVisitor; import dscanner.analysis.explicitly_annotated_unittests : ExplicitlyAnnotatedUnittestCheck; import dscanner.analysis.final_attribute : FinalAttributeChecker; import dscanner.analysis.has_public_example : HasPublicExampleCheck; +import dscanner.analysis.if_constraints_indent : IfConstraintsIndentCheck; import dscanner.analysis.ifelsesame : IfElseSameCheck; import dscanner.analysis.imports_sortedness : ImportSortednessCheck; import dscanner.analysis.incorrect_infinite_range : IncorrectInfiniteRangeCheck; @@ -332,6 +333,12 @@ MessageSet analyzeDmd(string fileName, ASTCodegen.Module m, const char[] moduleN config.mismatched_args_check == Check.skipTests && !ut ); + if (moduleName.shouldRunDmd!(IfConstraintsIndentCheck!ASTCodegen)(config)) + visitors ~= new IfConstraintsIndentCheck!ASTCodegen( + fileName, + config.if_constraints_indent == Check.skipTests && !ut + ); + foreach (visitor; visitors) { m.accept(visitor);