replace libdparse in local imports visitor (#45)
This commit is contained in:
parent
35deff3302
commit
697e59d7d7
|
|
@ -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;
|
{
|
||||||
|
// Look for import foo.bar : x or foo.bar : y = x
|
||||||
|
if (!i.isstatic && localImport && i.names.length == 0 && !i.aliasId)
|
||||||
|
{
|
||||||
|
addErrorMessage(cast(ulong) i.loc.linnum, cast(ulong) i.loc.charnum, KEY, MESSAGE);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
override void visit(const Declaration dec)
|
// Skip unittests for now
|
||||||
|
override void visit(AST.UnitTestDeclaration ud)
|
||||||
{
|
{
|
||||||
if (dec.importDeclaration is null)
|
|
||||||
{
|
|
||||||
dec.accept(this);
|
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
foreach (attr; dec.attributes)
|
|
||||||
{
|
|
||||||
if (attr.attribute == tok!"static")
|
|
||||||
isStatic = true;
|
|
||||||
}
|
|
||||||
dec.accept(this);
|
|
||||||
isStatic = false;
|
|
||||||
}
|
|
||||||
|
|
||||||
override void visit(const ImportDeclaration id)
|
|
||||||
{
|
|
||||||
if ((!isStatic && interesting) && (id.importBindings is null
|
|
||||||
|| 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.");
|
||||||
|
|
|
||||||
|
|
@ -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);
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue