diff --git a/src/analysis/builtin_property_names.d b/src/analysis/builtin_property_names.d index a5ff956..c5d62b0 100644 --- a/src/analysis/builtin_property_names.d +++ b/src/analysis/builtin_property_names.d @@ -29,8 +29,6 @@ class BuiltinPropertyNameCheck : BaseAnalyzer { alias visit = BaseAnalyzer.visit; - enum string KEY = "dscanner.confusing.builtin_property_names"; - this(string fileName) { super(fileName); @@ -47,7 +45,7 @@ class BuiltinPropertyNameCheck : BaseAnalyzer override void visit(const FunctionBody functionBody) { - int d = depth; + immutable int d = depth; scope(exit) depth = d; depth = 0; functionBody.accept(this); @@ -77,16 +75,18 @@ class BuiltinPropertyNameCheck : BaseAnalyzer private: + enum string KEY = "dscanner.confusing.builtin_property_names"; + string generateErrorMessage(string name) { - import std.string; + import std.string : format; return format("Avoid naming members '%s'. This can" ~ " confuse code that depends on the '.%s' property of a type.", name, name); } bool isBuiltinProperty(string name) { - import std.algorithm; + import std.algorithm : canFind; return builtinProperties.canFind(name); } @@ -98,7 +98,7 @@ private: unittest { - import analysis.config; + import analysis.config : StaticAnalysisConfig; StaticAnalysisConfig sac; sac.builtin_property_names_check = true; assertAnalyzerWarnings(q{ diff --git a/src/analysis/constructors.d b/src/analysis/constructors.d index 051a068..e9ad351 100644 --- a/src/analysis/constructors.d +++ b/src/analysis/constructors.d @@ -18,11 +18,11 @@ class ConstructorCheck : BaseAnalyzer override void visit(const ClassDeclaration classDeclaration) { - bool oldHasDefault = hasDefaultArgConstructor; - bool oldHasNoArg = hasNoArgConstructor; + immutable bool oldHasDefault = hasDefaultArgConstructor; + immutable bool oldHasNoArg = hasNoArgConstructor; hasNoArgConstructor = false; hasDefaultArgConstructor = false; - State prev = state; + immutable State prev = state; state = State.inClass; classDeclaration.accept(this); if (hasNoArgConstructor && hasDefaultArgConstructor) @@ -39,7 +39,7 @@ class ConstructorCheck : BaseAnalyzer override void visit(const StructDeclaration structDeclaration) { - State prev = state; + immutable State prev = state; state = State.inStruct; structDeclaration.accept(this); state = prev; diff --git a/src/analysis/helpers.d b/src/analysis/helpers.d index 0f5ca9a..454ec63 100644 --- a/src/analysis/helpers.d +++ b/src/analysis/helpers.d @@ -69,7 +69,7 @@ void assertAnalyzerWarnings(string code, const StaticAnalysisConfig config, stri foreach (rawWarning; rawWarnings[]) { // Skip the warning if it is on line zero - size_t rawLine = rawWarning.line; + immutable size_t rawLine = rawWarning.line; if (rawLine == 0) { stderr.writefln("!!! Skipping warning because it is on line zero:\n%s", rawWarning.message); @@ -89,8 +89,8 @@ void assertAnalyzerWarnings(string code, const StaticAnalysisConfig config, stri continue; // Skip if there is no comment or code - string codePart = codeLine.before("// "); - string commentPart = codeLine.after("// "); + immutable string codePart = codeLine.before("// "); + immutable string commentPart = codeLine.after("// "); if (!codePart.length || !commentPart.length) continue; @@ -107,7 +107,7 @@ void assertAnalyzerWarnings(string code, const StaticAnalysisConfig config, stri // No warning if (lineNo !in warnings) { - string errors = "Expected warning:\n%s\nFrom source code at (%s:?):\n%s".format( + immutable string errors = "Expected warning:\n%s\nFrom source code at (%s:?):\n%s".format( messages[lineNo], lineNo, codeLines[lineNo - line] @@ -117,7 +117,7 @@ void assertAnalyzerWarnings(string code, const StaticAnalysisConfig config, stri // Different warning else if (warnings[lineNo] != messages[lineNo]) { - string errors = "Expected warning:\n%s\nBut was:\n%s\nFrom source code at (%s:?):\n%s".format( + immutable string errors = "Expected warning:\n%s\nBut was:\n%s\nFrom source code at (%s:?):\n%s".format( messages[lineNo], warnings[lineNo], lineNo, @@ -143,7 +143,7 @@ void assertAnalyzerWarnings(string code, const StaticAnalysisConfig config, stri } if (unexpectedWarnings.length) { - string message = "Unexpected warnings:\n" ~ unexpectedWarnings.join("\n"); + immutable string message = "Unexpected warnings:\n" ~ unexpectedWarnings.join("\n"); throw new core.exception.AssertError(message, file, line); } } diff --git a/src/analysis/immutable_finder.d b/src/analysis/immutable_finder.d index dcfe723..8cca3de 100644 --- a/src/analysis/immutable_finder.d +++ b/src/analysis/immutable_finder.d @@ -54,6 +54,8 @@ class ImmutableFinder:BaseAnalyzer { foreach (d; dec.declarators) { + if (initializedFromCast(d.initializer)) + continue; tree[$ - 1].insert(new VariableInfo(d.name.text, d.name.line, d.name.column)); } @@ -68,9 +70,13 @@ class ImmutableFinder:BaseAnalyzer && autoDeclaration.storageClass.token != tok!"enum" && autoDeclaration.storageClass.token != tok!"immutable")) { - foreach (id; autoDeclaration.identifiers) + foreach (size_t i, id; autoDeclaration.identifiers) + { + if (initializedFromCast(autoDeclaration.initializers[i])) + continue; tree[$ - 1].insert(new VariableInfo(id.text, id.line, id.column)); + } } autoDeclaration.accept(this); } @@ -130,6 +136,17 @@ class ImmutableFinder:BaseAnalyzer unary.accept(this); } + override void visit(const ForeachStatement foreachStatement) + { + if (foreachStatement.low !is null) + { + interest++; + foreachStatement.low.accept(this); + interest--; + } + foreachStatement.declarationOrStatement.accept(this); + } + private: template PartsMightModify(T) @@ -156,6 +173,28 @@ private: } } + bool initializedFromCast(const Initializer initializer) + { + import std.typecons : scoped; + + static class CastFinder : ASTVisitor + { + alias visit = ASTVisitor.visit; + override void visit(const CastExpression castExpression) + { + foundCast = true; + castExpression.accept(this); + } + bool foundCast = false; + } + + if (initializer is null) + return false; + auto finder = scoped!CastFinder(); + finder.visit(initializer); + return finder.foundCast; + } + bool canFindImmutable(const Declaration dec) { import std.algorithm : canFind, map; @@ -195,7 +234,7 @@ private: foreach (vi; tree[$ - 1]) { immutable string errorMessage = "Variable " ~ vi.name - ~ " could have been declared immutable"; + ~ " is never modified and could have been declared immutable."; addErrorMessage(vi.line, vi.column, "dscanner.suspicious.could_be_immutable", errorMessage); } diff --git a/src/analysis/length_subtraction.d b/src/analysis/length_subtraction.d index a6141d5..63ad720 100644 --- a/src/analysis/length_subtraction.d +++ b/src/analysis/length_subtraction.d @@ -29,8 +29,8 @@ class LengthSubtractionCheck : BaseAnalyzer { if (addExpression.operator == tok!"-") { - UnaryExpression l = cast(UnaryExpression) addExpression.left; - UnaryExpression r = cast(UnaryExpression) addExpression.right; + const UnaryExpression l = cast(const UnaryExpression) addExpression.left; + const UnaryExpression r = cast(const UnaryExpression) addExpression.right; if (l is null || r is null) { // stderr.writeln(__FILE__, " ", __LINE__); diff --git a/src/analysis/opequals_without_tohash.d b/src/analysis/opequals_without_tohash.d index fa8c7d2..44d58ba 100644 --- a/src/analysis/opequals_without_tohash.d +++ b/src/analysis/opequals_without_tohash.d @@ -57,7 +57,7 @@ class OpEqualsWithoutToHashCheck : BaseAnalyzer continue; // Check if opEquals or toHash - string methodName = declaration.functionDeclaration.name.text; + immutable string methodName = declaration.functionDeclaration.name.text; if (methodName == "opEquals") hasOpEquals = true; else if (methodName == "toHash") diff --git a/src/analysis/unused.d b/src/analysis/unused.d index cb7bf86..b8e210c 100644 --- a/src/analysis/unused.d +++ b/src/analysis/unused.d @@ -44,7 +44,7 @@ class UnusedVariableCheck : BaseAnalyzer pushScope(); if (functionDec.functionBody !is null) { - bool ias = inAggregateScope; + immutable bool ias = inAggregateScope; inAggregateScope = false; if (!isOverride) functionDec.parameters.accept(this); diff --git a/src/main.d b/src/main.d index b3ba04e..c700367 100644 --- a/src/main.d +++ b/src/main.d @@ -133,7 +133,7 @@ int run(string[] args) } else if (tokenDump || highlight) { - bool usingStdin = args.length == 1; + immutable bool usingStdin = args.length == 1; ubyte[] bytes = usingStdin ? readStdin() : readFile(args[1]); LexerConfig config; config.stringBehavior = StringBehavior.source; @@ -211,7 +211,7 @@ int run(string[] args) } else if (imports) { - string[] fileNames = usingStdin ? ["stdin"] : args[1 .. $]; + const string[] fileNames = usingStdin ? ["stdin"] : args[1 .. $]; LexerConfig config; config.stringBehavior = StringBehavior.source; auto visitor = new ImportPrinter; @@ -236,12 +236,6 @@ int run(string[] args) auto tokens = getTokensForParser( usingStdin ? readStdin() : readFile(args[1]), config, &cache); -// writeln("text blank\tindex\tline\tcolumn\ttype\tcomment"); -// foreach (token; tokens) -// { -// writefln("<<%20s>>%b\t%d\t%d\t%d\t%d\t%s", token.text is null ? str(token.type) : token.text, -// token.text !is null, token.index, token.line, token.column, token.type, token.comment); -// } auto mod = parseModule(tokens, fileName, null, &doNothing); if (ast) @@ -387,6 +381,7 @@ string getConfigurationLocation() { version (useXDG) { + int x; import std.process; string configDir = environment.get("XDG_CONFIG_HOME", null); if (configDir is null)