replace libdparse in local imports visitor (#45)

This commit is contained in:
lucica28 2022-12-01 14:58:52 +02:00 committed by Vladiwostok
parent 010ac1d3b7
commit a3a5982e2c
2 changed files with 59 additions and 60 deletions

View File

@ -5,100 +5,100 @@
module dscanner.analysis.local_imports; module dscanner.analysis.local_imports;
import std.stdio;
import dparse.ast;
import dparse.lexer;
import dscanner.analysis.base; import dscanner.analysis.base;
import dscanner.analysis.helpers; import dscanner.analysis.helpers;
import dsymbol.scope_;
import std.stdio : writeln;
/** /**
* Checks for local imports that import all symbols. * Checks for local imports that import all symbols.
* See_also: $(LINK https://issues.dlang.org/show_bug.cgi?id=10378) * See_also: $(LINK https://issues.dlang.org/show_bug.cgi?id=10378)
*/ */
final class LocalImportCheck : BaseAnalyzer extern(C++) class LocalImportCheck(AST) : BaseAnalyzerDmd
{ {
alias visit = BaseAnalyzer.visit;
mixin AnalyzerInfo!"local_import_check"; mixin AnalyzerInfo!"local_import_check";
alias visit = BaseAnalyzerDmd.visit;
/** mixin ScopedVisit!(AST.FuncDeclaration);
* Construct with the given file name. mixin ScopedVisit!(AST.IfStatement);
*/ mixin ScopedVisit!(AST.WhileStatement);
this(BaseAnalyzerArguments args) mixin ScopedVisit!(AST.ForStatement);
mixin ScopedVisit!(AST.ForeachStatement);
mixin ScopedVisit!(AST.ClassDeclaration);
mixin ScopedVisit!(AST.StructDeclaration);
extern(D) this(string fileName)
{ {
super(args); super(fileName);
this.localImport = false;
} }
mixin visitThing!StructBody; override void visit(AST.Import i)
mixin visitThing!BlockStatement;
override void visit(const Declaration dec)
{ {
if (dec.importDeclaration is null) // Look for import foo.bar : x or foo.bar : y = x
if (!i.isstatic && localImport && i.names.length == 0 && !i.aliasId)
{ {
dec.accept(this); addErrorMessage(cast(ulong) i.loc.linnum, cast(ulong) i.loc.charnum, KEY, MESSAGE);
return;
} }
foreach (attr; dec.attributes)
{
if (attr.attribute == tok!"static")
isStatic = true;
}
dec.accept(this);
isStatic = false;
} }
override void visit(const ImportDeclaration id) // Skip unittests for now
override void visit(AST.UnitTestDeclaration ud)
{ {
if ((!isStatic && interesting) && (id.importBindings is null return;
|| id.importBindings.importBinds.length == 0))
{
foreach (singleImport; id.singleImports)
{
if (singleImport.rename.text.length == 0)
{
addErrorMessage(singleImport,
KEY, "Local imports should specify"
~ " the symbols being imported to avoid hiding local symbols.");
}
}
}
} }
private: private:
template ScopedVisit(NodeType)
enum string KEY = "dscanner.suspicious.local_imports";
mixin template visitThing(T)
{ {
override void visit(const T thing) override void visit(NodeType n)
{ {
const b = interesting; bool prevState = localImport;
interesting = true; localImport = true;
thing.accept(this); super.visit(n);
interesting = b; localImport = prevState;
} }
} }
bool interesting; bool localImport;
bool isStatic; enum KEY = "dscanner.suspicious.local_imports";
enum MESSAGE = "Local imports should specify the symbols being imported to avoid hiding local symbols.";
} }
unittest unittest
{ {
import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig; import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
import std.stdio : stderr;
StaticAnalysisConfig sac = disabledConfig(); StaticAnalysisConfig sac = disabledConfig();
sac.local_import_check = Check.enabled; sac.local_import_check = Check.enabled;
assertAnalyzerWarnings(q{
void testLocalImport() assertAnalyzerWarningsDMD(q{
import std.experimental;
void foo()
{ {
import std.stdio; /+ import std.stdio; // [warn]: Local imports should specify the symbols being imported to avoid hiding local symbols.
^^^^^^^^^ [warn]: Local imports should specify the symbols being imported to avoid hiding local symbols. +/
import std.fish : scales, head; import std.fish : scales, head;
import DAGRON = std.experimental.dragon; import DAGRON = std.experimental.dragon;
if (1)
{
import foo.bar; // [warn]: Local imports should specify the symbols being imported to avoid hiding local symbols.
}
else
{
import foo.bar; // [warn]: Local imports should specify the symbols being imported to avoid hiding local symbols.
}
foreach (i; [1, 2, 3])
{
import foo.bar; // [warn]: Local imports should specify the symbols being imported to avoid hiding local symbols.
import std.stdio : writeln;
}
} }
import std.experimental.dragon;
}c, sac); }c, sac);
stderr.writeln("Unittest for LocalImportCheck passed."); stderr.writeln("Unittest for LocalImportCheck passed.");

View File

@ -868,10 +868,6 @@ private BaseAnalyzer[] getAnalyzersForModuleAndConfig(string fileName,
checks ~= new LabelVarNameCheck(args.setSkipTests( checks ~= new LabelVarNameCheck(args.setSkipTests(
analysisConfig.label_var_same_name_check == Check.skipTests && !ut)); analysisConfig.label_var_same_name_check == Check.skipTests && !ut));
if (moduleName.shouldRun!LocalImportCheck(analysisConfig))
checks ~= new LocalImportCheck(args.setSkipTests(
analysisConfig.local_import_check == Check.skipTests && !ut));
if (moduleName.shouldRun!LogicPrecedenceCheck(analysisConfig)) if (moduleName.shouldRun!LogicPrecedenceCheck(analysisConfig))
checks ~= new LogicPrecedenceCheck(args.setSkipTests( checks ~= new LogicPrecedenceCheck(args.setSkipTests(
analysisConfig.logical_precedence_check == Check.skipTests && !ut)); analysisConfig.logical_precedence_check == Check.skipTests && !ut));
@ -1323,6 +1319,9 @@ MessageSet analyzeDmd(string fileName, ASTBase.Module m, const char[] moduleName
if (moduleName.shouldRunDmd!(ConstructorCheck!ASTBase)(config)) if (moduleName.shouldRunDmd!(ConstructorCheck!ASTBase)(config))
visitors ~= new ConstructorCheck!ASTBase(fileName); visitors ~= new ConstructorCheck!ASTBase(fileName);
if (moduleName.shouldRunDmd!(LocalImportCheck!ASTBase)(config))
visitors ~= new LocalImportCheck!ASTBase(fileName);
foreach (visitor; visitors) foreach (visitor; visitors)
{ {
m.accept(visitor); m.accept(visitor);