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
This commit is contained in:
Vladiwostok 2024-11-08 09:35:10 +02:00 committed by Vladiwostok
parent c276428331
commit 7e50946351
3 changed files with 124 additions and 162 deletions

View file

@ -4,194 +4,132 @@
module dscanner.analysis.if_constraints_indent; module dscanner.analysis.if_constraints_indent;
import dparse.lexer;
import dparse.ast;
import dscanner.analysis.base; import dscanner.analysis.base;
import dsymbol.scope_ : Scope; import dmd.tokens : Token, TOK;
import std.algorithm.iteration : filter;
import std.range;
/** /**
Checks whether all if constraints have the same indention as their declaration. 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"; mixin AnalyzerInfo!"if_constraints_indent";
/// private enum string KEY = "dscanner.style.if_constraints_indent";
this(BaseAnalyzerArguments args) private enum string MSG = "If constraints should have the same indentation as the function";
private Token[] tokens;
extern (D) this(string fileName, bool skipTests = false)
{ {
super(args); super(fileName, skipTests);
lexFile();
// convert tokens to a list of token starting positions per line
// 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) private void lexFile()
{ {
if (decl.constraint !is null) import dscanner.utils : readFile;
checkConstraintSpace(decl.constraint, decl.name); import dmd.errorsink : ErrorSinkNull;
import dmd.globals : global;
import dmd.lexer : Lexer;
auto bytes = readFile(fileName) ~ '\0';
__gshared ErrorSinkNull errorSinkNull;
if (!errorSinkNull)
errorSinkNull = new ErrorSinkNull;
scope lexer = new Lexer(null, cast(char*) bytes, 0, bytes.length, 0, 0, errorSinkNull, &global.compileEnv);
do
{
lexer.nextToken();
tokens ~= lexer.token;
}
while (lexer.token.value != TOK.endOfFile);
} }
override void visit(const InterfaceDeclaration decl) override void visit(AST.TemplateDeclaration templateDecl)
{ {
if (decl.constraint !is null) import std.array : array;
checkConstraintSpace(decl.constraint, decl.name); import std.algorithm : filter;
} import std.range : front, retro;
super.visit(templateDecl);
override void visit(const ClassDeclaration decl) if (templateDecl.constraint is null || templateDecl.members is null)
{ return;
if (decl.constraint !is null)
checkConstraintSpace(decl.constraint, decl.name);
}
override void visit(const TemplateDeclaration decl) auto firstTemplateToken = tokens.filter!(t => t.loc.linnum == templateDecl.loc.linnum)
{ .filter!(t => t.value != TOK.whitespace)
if (decl.constraint !is null) .front;
checkConstraintSpace(decl.constraint, decl.name); uint templateLine = firstTemplateToken.loc.linnum;
} uint templateCol = firstTemplateToken.loc.charnum;
override void visit(const UnionDeclaration decl) auto constraintToken = tokens.filter!(t => t.loc.fileOffset <= templateDecl.constraint.loc.fileOffset)
{ .array()
if (decl.constraint !is null) .retro()
checkConstraintSpace(decl.constraint, decl.name); .filter!(t => t.value == TOK.if_)
} .front;
uint constraintLine = constraintToken.loc.linnum;
uint constraintCol = constraintToken.loc.charnum;
override void visit(const StructDeclaration decl) if (templateLine == constraintLine || templateCol != constraintCol)
{ addErrorMessage(cast(ulong) constraintLine, cast(ulong) constraintCol, KEY, MSG);
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);
}
void checkConstraintSpace(const Constraint constraint, size_t line)
{
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);
} }
} }
unittest unittest
{ {
import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig; import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
import dscanner.analysis.helpers : assertAnalyzerWarnings; import dscanner.analysis.helpers : assertAnalyzerWarningsDMD;
import std.format : format; import std.format : format;
import std.stdio : stderr; import std.stdio : stderr;
StaticAnalysisConfig sac = disabledConfig(); StaticAnalysisConfig sac = disabledConfig();
sac.if_constraints_indent = Check.enabled; 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 +/
{}
}c.format(
IfConstraintsIndentCheck.MESSAGE,
), sac);
assertAnalyzerWarnings(q{
void foo(R)(R r) void foo(R)(R r)
if (R == null) if (R == null)
{} {}
void foo(R)(R r) void foo(R)(R r)
if (R == null) /+ if (R == null) // [warn]: %s
^^ [warn]: %s +/ {}
}c.format(MSG), sac);
assertAnalyzerWarningsDMD(q{
void foo(R)(R r)
if (R == null)
{} {}
void foo(R)(R r) void foo(R)(R r)
if (R == null) /+ if (R == null) // [warn]: %s
^^^ [warn]: %s +/
{} {}
}c.format(
IfConstraintsIndentCheck.MESSAGE,
IfConstraintsIndentCheck.MESSAGE,
), sac);
assertAnalyzerWarnings(q{ void foo(R)(R r)
if (R == null) // [warn]: %s
{}
}c.format(MSG, MSG), sac);
assertAnalyzerWarningsDMD(q{
struct Foo(R) struct Foo(R)
if (R == null) if (R == null)
{} {}
struct Foo(R) struct Foo(R)
if (R == null) /+ if (R == null) // [warn]: %s
^^ [warn]: %s +/
{} {}
struct Foo(R) struct Foo(R)
if (R == null) /+ if (R == null) // [warn]: %s
^^^ [warn]: %s +/
{} {}
}c.format( }c.format(MSG, MSG), sac);
IfConstraintsIndentCheck.MESSAGE,
IfConstraintsIndentCheck.MESSAGE,
), sac);
// test example from Phobos // test example from Phobos
assertAnalyzerWarnings(q{ assertAnalyzerWarningsDMD(q{
Num abs(Num)(Num x) @safe pure nothrow Num abs(Num)(Num x) @safe pure nothrow
if (is(typeof(Num.init >= 0)) && is(typeof(-Num.init)) && if (is(typeof(Num.init >= 0)) && is(typeof(-Num.init)) &&
!(is(Num* : const(ifloat*)) || is(Num* : const(idouble*)) !(is(Num* : const(ifloat*)) || is(Num* : const(idouble*))
@ -205,7 +143,7 @@ if (is(typeof(Num.init >= 0)) && is(typeof(-Num.init)) &&
}, sac); }, sac);
// weird constraint formatting // weird constraint formatting
assertAnalyzerWarnings(q{ assertAnalyzerWarningsDMD(q{
struct Foo(R) struct Foo(R)
if if
(R == null) (R == null)
@ -217,8 +155,7 @@ if (is(typeof(Num.init >= 0)) && is(typeof(-Num.init)) &&
{} {}
struct Foo(R) struct Foo(R)
if /+ if // [warn]: %s
^^ [warn]: %s +/
(R == null) (R == null)
{} {}
@ -234,39 +171,61 @@ if /+
{} {}
struct Foo(R) struct Foo(R)
if ( /+ if ( // [warn]: %s
^^^ [warn]: %s +/
R == null R == null
) {} ) {}
}c.format( }c.format(MSG, MSG), sac);
IfConstraintsIndentCheck.MESSAGE,
IfConstraintsIndentCheck.MESSAGE,
), sac);
// constraint on the same line // constraint on the same line
assertAnalyzerWarnings(q{ assertAnalyzerWarningsDMD(q{
struct CRC(uint N, ulong P) if (N == 32 || N == 64) /+ struct CRC(uint N, ulong P) if (N == 32 || N == 64) // [warn]: %s
^^ [warn]: %s +/
{} {}
}c.format( }c.format(MSG), sac);
IfConstraintsIndentCheck.MESSAGE,
), sac); assertAnalyzerWarningsDMD(q{
private template sharedToString(alias field)
if (is(typeof(field) == shared))
{
static immutable sharedToString = typeof(field).stringof;
}
}c, sac);
assertAnalyzerWarningsDMD(q{
private union EndianSwapper(T)
if (canSwapEndianness!T)
{
T value;
}
}c, sac);
assertAnalyzerWarningsDMD(q{
void test(alias matchFn)()
{
auto baz(Cap)(Cap m)
if (is(Cap == Captures!(Cap.String)))
{
return toUpper(m.hit);
}
}
}c, 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."); stderr.writeln("Unittest for IfConstraintsIndentCheck passed.");
} }
@("issue #829")
unittest
{
import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
import dscanner.analysis.helpers : assertAnalyzerWarnings;
import std.format : format;
import std.stdio : stderr;
StaticAnalysisConfig sac = disabledConfig();
sac.if_constraints_indent = Check.enabled;
assertAnalyzerWarnings(`void foo() {
f;
}`, sac);
}

View file

@ -663,10 +663,6 @@ BaseAnalyzer[] getAnalyzersForModuleAndConfig(string fileName,
checks ~= new UndocumentedDeclarationCheck(args.setSkipTests( checks ~= new UndocumentedDeclarationCheck(args.setSkipTests(
analysisConfig.undocumented_declaration_check == Check.skipTests && !ut)); 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; return checks;
} }

View file

@ -25,6 +25,7 @@ import dscanner.analysis.enumarrayliteral : EnumArrayVisitor;
import dscanner.analysis.explicitly_annotated_unittests : ExplicitlyAnnotatedUnittestCheck; import dscanner.analysis.explicitly_annotated_unittests : ExplicitlyAnnotatedUnittestCheck;
import dscanner.analysis.final_attribute : FinalAttributeChecker; import dscanner.analysis.final_attribute : FinalAttributeChecker;
import dscanner.analysis.has_public_example : HasPublicExampleCheck; import dscanner.analysis.has_public_example : HasPublicExampleCheck;
import dscanner.analysis.if_constraints_indent : IfConstraintsIndentCheck;
import dscanner.analysis.ifelsesame : IfElseSameCheck; import dscanner.analysis.ifelsesame : IfElseSameCheck;
import dscanner.analysis.imports_sortedness : ImportSortednessCheck; import dscanner.analysis.imports_sortedness : ImportSortednessCheck;
import dscanner.analysis.incorrect_infinite_range : IncorrectInfiniteRangeCheck; 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 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) foreach (visitor; visitors)
{ {
m.accept(visitor); m.accept(visitor);