diff --git a/analysis/config.d b/analysis/config.d index bf06b3f..18331cf 100644 --- a/analysis/config.d +++ b/analysis/config.d @@ -56,4 +56,7 @@ struct StaticAnalysisConfig @INI("Checks that opEquals and toHash are both defined or neither are defined") bool opequals_tohash_check; + + @INI("Checks for subtraction from .length properties") + bool length_subtraction_check; } diff --git a/analysis/length_subtraction.d b/analysis/length_subtraction.d new file mode 100644 index 0000000..e8f8b25 --- /dev/null +++ b/analysis/length_subtraction.d @@ -0,0 +1,73 @@ +// Copyright Brian Schott (Sir Alaran) 2014. +// Distributed under the Boost Software License, Version 1.0. +// (See accompanying file LICENSE_1_0.txt or copy at +// http://www.boost.org/LICENSE_1_0.txt) + +module analysis.length_subtraction; + +import std.stdio; + +import std.d.ast; +import std.d.lexer; +import analysis.base; +import analysis.helpers; + + +/** + * Checks for subtraction from a .length property. This is usually a bug. + */ +class LengthSubtractionCheck : BaseAnalyzer +{ + alias visit = BaseAnalyzer.visit; + + this(string fileName) + { + super(fileName); + } + + override void visit(const AddExpression addExpression) + { + if (addExpression.operator == tok!"-") + { + UnaryExpression l = cast(UnaryExpression) addExpression.left; + UnaryExpression r = cast(UnaryExpression) addExpression.right; + if (l is null || r is null) + { +// stderr.writeln(__FILE__, " ", __LINE__); + goto end; + } + if (r.primaryExpression is null || r.primaryExpression.primary.type != tok!"intLiteral") + { +// stderr.writeln(__FILE__, " ", __LINE__); + goto end; + } + if (l.identifierOrTemplateInstance is null + || l.identifierOrTemplateInstance.identifier.text != "length") + { +// stderr.writeln(__FILE__, " ", __LINE__); + goto end; + } + const(Token) token = l.identifierOrTemplateInstance.identifier; + addErrorMessage(token.line, token.column, + "Avoid subtracting from '.length' as it may be unsigned."); + } + end: + addExpression.accept(this); + } +} + +unittest +{ + import analysis.config; + StaticAnalysisConfig sac; + sac.if_else_same_check = true; + assertAnalyzerWarnings(q{ + void testSizeT() + { + if (i < a.length - 1) // [warn]: Avoid subtracting from '.length' as it may be unsigned. + writeln("something"); + } + }c, sac); + stderr.writeln("Unittest for IfElseSameCheck passed."); +} + diff --git a/analysis/run.d b/analysis/run.d index e5fba7d..3203e8f 100644 --- a/analysis/run.d +++ b/analysis/run.d @@ -25,6 +25,7 @@ import analysis.constructors; import analysis.unused; import analysis.duplicate_attribute; import analysis.opequals_without_tohash; +import analysis.length_subtraction; void messageFunction(string fileName, size_t line, size_t column, string message, bool isError) @@ -94,6 +95,7 @@ string[] analyze(string fileName, ubyte[] code, StaticAnalysisConfig analysisCon if (analysisConfig.unused_variable_check) checks ~= new UnusedVariableCheck(fileName); if (analysisConfig.duplicate_attribute) checks ~= new DuplicateAttributeCheck(fileName); if (analysisConfig.opequals_tohash_check) checks ~= new OpEqualsWithoutToHashCheck(fileName); + if (analysisConfig.length_subtraction_check) checks ~= new LengthSubtractionCheck(fileName); foreach (check; checks) { diff --git a/astprinter.d b/astprinter.d index 8dd825e..292957c 100644 --- a/astprinter.d +++ b/astprinter.d @@ -61,12 +61,12 @@ class XMLPrinter : ASTVisitor { output.writeln(""); output.writeln(""); - andAndExpression.left.accept(this); + visit(andAndExpression.left); output.writeln(""); if (andAndExpression.right !is null) { output.writeln(""); - andAndExpression.right.accept(this); + visit(andAndExpression.right); output.writeln(""); } output.writeln(""); @@ -76,12 +76,12 @@ class XMLPrinter : ASTVisitor { output.writeln(""); output.writeln(""); - andExpression.left.accept(this); + visit(andExpression.left); output.writeln(""); if (andExpression.right !is null) { output.writeln(""); - andExpression.right.accept(this); + visit(andExpression.right); output.writeln(""); } output.writeln(""); @@ -441,10 +441,10 @@ class XMLPrinter : ASTVisitor { output.writeln(""); output.writeln(""); - equalExpression.left.accept(this); + visit(equalExpression.left); output.writeln(""); output.writeln(""); - equalExpression.right.accept(this); + visit(equalExpression.right); output.writeln(""); output.writeln(""); } @@ -956,12 +956,12 @@ class XMLPrinter : ASTVisitor { output.writeln(""); output.writeln(""); - powExpression.left.accept(this); + visit(powExpression.left); output.writeln(""); if (powExpression.right !is null) { output.writeln(""); - powExpression.right.accept(this); + visit(powExpression.right); output.writeln(""); } output.writeln(""); @@ -997,10 +997,10 @@ class XMLPrinter : ASTVisitor output.writeln(""); output.writeln(""); - relExpression.left.accept(this); + visit(relExpression.left); output.writeln(""); output.writeln(""); - relExpression.right.accept(this); + visit(relExpression.right); output.writeln(""); output.writeln(""); } @@ -1037,10 +1037,10 @@ class XMLPrinter : ASTVisitor output.writeln(""); output.writeln(""); - shiftExpression.left.accept(this); + visit(shiftExpression.left); output.writeln(""); output.writeln(""); - shiftExpression.right.accept(this); + visit(shiftExpression.right); output.writeln(""); output.writeln(""); } @@ -1496,12 +1496,12 @@ class XMLPrinter : ASTVisitor { output.writeln(""); output.writeln(""); - xorExpression.left.accept(this); + visit(xorExpression.left); output.writeln(""); if (xorExpression.right !is null) { output.writeln(""); - xorExpression.right.accept(this); + visit(xorExpression.right); output.writeln(""); } output.writeln("");