From cb073c3cf0906ba74f80230b7ecae6ed02c0daf0 Mon Sep 17 00:00:00 2001 From: Sebastian Wilzbach Date: Mon, 12 Jun 2017 22:36:09 +0200 Subject: [PATCH] Add check for if constraint indendation --- README.md | 1 + src/dscanner/analysis/config.d | 3 + src/dscanner/analysis/if_constraints_indent.d | 233 ++++++++++++++++++ src/dscanner/analysis/run.d | 5 + 4 files changed, 242 insertions(+) create mode 100644 src/dscanner/analysis/if_constraints_indent.d diff --git a/README.md b/README.md index f51008e..54dca6b 100644 --- a/README.md +++ b/README.md @@ -140,6 +140,7 @@ Note that the "--skipTests" option is the equivalent of changing each * Redundant visibility attributes * Public declarations without a documented unittest. By default disabled. * Asserts without an explanatory message. By default disabled. +* Indentation of if constraints #### Wishlist diff --git a/src/dscanner/analysis/config.d b/src/dscanner/analysis/config.d index 8917c67..79e5c99 100644 --- a/src/dscanner/analysis/config.d +++ b/src/dscanner/analysis/config.d @@ -192,6 +192,9 @@ struct StaticAnalysisConfig @INI("Check for asserts without an explanatory message") string assert_without_msg = Check.disabled; + @INI("Check indent of if constraints") + string if_constraints_indent = Check.disabled; + @INI("Module-specific filters") ModuleFilters filters; } diff --git a/src/dscanner/analysis/if_constraints_indent.d b/src/dscanner/analysis/if_constraints_indent.d new file mode 100644 index 0000000..453d0b1 --- /dev/null +++ b/src/dscanner/analysis/if_constraints_indent.d @@ -0,0 +1,233 @@ +// 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 dscanner.analysis.if_constraints_indent; + +import dparse.lexer; +import dparse.ast; +import dscanner.analysis.base : BaseAnalyzer, Message; +import dsymbol.scope_ : Scope; + +import std.algorithm.iteration : filter; +import std.range; + +/** +Checks whether all if constraints have the same indention as their declaration. +*/ +class IfConstraintsIndentCheck : BaseAnalyzer +{ + /// + this(string fileName, const(Token)[] tokens, bool skipTests = false) + { + super(fileName, null, skipTests); + + // libdparse columns start at 1 + foreach (t; tokens) + { + while (firstSymbolAtLine.length < t.line - 1) + firstSymbolAtLine ~= Pos(1); + + if (firstSymbolAtLine.length < t.line) + firstSymbolAtLine ~= Pos(t.column, t.type == tok!"if"); + } + } + + override void visit(const FunctionDeclaration decl) + { + if (decl.constraint !is null) + checkConstraintSpace(decl.constraint, decl.name); + } + + override void visit(const InterfaceDeclaration decl) + { + if (decl.constraint !is null) + checkConstraintSpace(decl.constraint, decl.name); + } + + + override void visit(const ClassDeclaration decl) + { + if (decl.constraint !is null) + checkConstraintSpace(decl.constraint, decl.name); + } + + override void visit(const TemplateDeclaration decl) + { + if (decl.constraint !is null) + checkConstraintSpace(decl.constraint, decl.name); + } + + override void visit(const UnionDeclaration decl) + { + if (decl.constraint !is null) + checkConstraintSpace(decl.constraint, decl.name); + } + + override void visit(const StructDeclaration decl) + { + 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; + 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) + { + // 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); + + // no hit = constraint is on the same line + if (r.empty) + addErrorMessage(constraint.expression.line, constraint.expression.column, KEY, MESSAGE); + else if (pDecl.column != r.front.column) + addErrorMessage(constraint.expression.line, constraint.expression.column, KEY, MESSAGE); + } +} + +unittest +{ + import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig; + import dscanner.analysis.helpers; + import std.format : format; + import std.stdio : stderr; + + StaticAnalysisConfig sac = disabledConfig(); + sac.if_constraints_indent = Check.enabled; + + assertAnalyzerWarnings(q{ +void foo(R)(R r) +if (R == int) +{} + +void foo(R)(R r) + if (R == int) // [warn]: %s +{} + }c.format( + IfConstraintsIndentCheck.MESSAGE, + ), sac); + + assertAnalyzerWarnings(q{ + void foo(R)(R r) + if (R == int) + {} + + void foo(R)(R r) +if (R == int) // [warn]: %s + {} + + void foo(R)(R r) + if (R == int) // [warn]: %s + {} + }c.format( + IfConstraintsIndentCheck.MESSAGE, + IfConstraintsIndentCheck.MESSAGE, + ), sac); + + assertAnalyzerWarnings(q{ + struct Foo(R) + if (R == int) + {} + + struct Foo(R) +if (R == int) // [warn]: %s + {} + + struct Foo(R) + if (R == int) // [warn]: %s + {} + }c.format( + IfConstraintsIndentCheck.MESSAGE, + IfConstraintsIndentCheck.MESSAGE, + ), sac); + + // test example from Phobos + assertAnalyzerWarnings(q{ +Num abs(Num)(Num x) @safe pure nothrow +if (is(typeof(Num.init >= 0)) && is(typeof(-Num.init)) && + !(is(Num* : const(ifloat*)) || is(Num* : const(idouble*)) + || is(Num* : const(ireal*)))) +{ + static if (isFloatingPoint!(Num)) + return fabs(x); + else + return x >= 0 ? x : -x; +} + }, sac); + + // weird constraint formatting + assertAnalyzerWarnings(q{ + struct Foo(R) + if + (R == int) + {} + + struct Foo(R) + if + (R == int) + {} + + struct Foo(R) +if + (R == int) // [warn]: %s + {} + + struct Foo(R) + if ( + R == int) + {} + + struct Foo(R) + if ( + R == int + ) + {} + + struct Foo(R) + if ( + R == int // [warn]: %s + ) {} + }c.format( + IfConstraintsIndentCheck.MESSAGE, + IfConstraintsIndentCheck.MESSAGE, + ), sac); + + // constraint on the same line + assertAnalyzerWarnings(q{ + struct CRC(uint N, ulong P) if (N == 32 || N == 64) // [warn]: %s + {} + }c.format( + IfConstraintsIndentCheck.MESSAGE, + ), sac); + + stderr.writeln("Unittest for IfConstraintsIndentCheck passed."); +} diff --git a/src/dscanner/analysis/run.d b/src/dscanner/analysis/run.d index 57d8469..005b288 100644 --- a/src/dscanner/analysis/run.d +++ b/src/dscanner/analysis/run.d @@ -71,6 +71,7 @@ import dscanner.analysis.allman; import dscanner.analysis.redundant_attributes; import dscanner.analysis.has_public_example; import dscanner.analysis.assert_without_msg; +import dscanner.analysis.if_constraints_indent; import dsymbol.string_interning : internString; import dsymbol.scope_; @@ -506,6 +507,10 @@ MessageSet analyze(string fileName, const Module m, const StaticAnalysisConfig a checks ~= new AssertWithoutMessageCheck(fileName, moduleScope, analysisConfig.assert_without_msg == Check.skipTests && !ut); + if (moduleName.shouldRun!"if_constraints_indent"(analysisConfig)) + checks ~= new IfConstraintsIndentCheck(fileName, tokens, + analysisConfig.if_constraints_indent == Check.skipTests && !ut); + version (none) if (moduleName.shouldRun!"redundant_if_check"(analysisConfig)) checks ~= new IfStatementCheck(fileName, moduleScope,