From b43c8f45cf430f7d2529ef5c4b313c2aad75c147 Mon Sep 17 00:00:00 2001
From: Robert Schadek <rschadek@symmetryinvestments.com>
Date: Fri, 1 Sep 2023 15:39:05 +0100
Subject: [PATCH] Always Check Curly
Check that if|else|for|foreach|while|do|try|catch
are always followed by a BlockStatement aka. { }
closer
can not get the test to work
try to get the AutoFix in place
maybe a fix
nicer messages
some formatting
more tinkering
still nothing
autofix work now
AutoFix name
message to message_postfix
---
src/dscanner/analysis/always_curly.d | 227 +++++++++++++++++++++++++++
src/dscanner/analysis/config.d | 3 +
src/dscanner/analysis/run.d | 5 +
3 files changed, 235 insertions(+)
create mode 100644 src/dscanner/analysis/always_curly.d
diff --git a/src/dscanner/analysis/always_curly.d b/src/dscanner/analysis/always_curly.d
new file mode 100644
index 0000000..5653d61
--- /dev/null
+++ b/src/dscanner/analysis/always_curly.d
@@ -0,0 +1,227 @@
+// 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.always_curly;
+
+import dparse.lexer;
+import dparse.ast;
+import dscanner.analysis.base;
+import dsymbol.scope_ : Scope;
+
+import std.array : back, front;
+import std.algorithm;
+import std.range;
+import std.stdio;
+
+final class AlwaysCurlyCheck : BaseAnalyzer
+{
+ mixin AnalyzerInfo!"always_curly_check";
+
+ alias visit = BaseAnalyzer.visit;
+
+ ///
+ this(string fileName, const(Token)[] tokens, bool skipTests = false)
+ {
+ super(fileName, null, skipTests);
+ }
+
+ void test(L, B)(L loc, B s, string stmtKind)
+ {
+ if (!is(s == BlockStatement))
+ {
+ if (!s.tokens.empty)
+ {
+ AutoFix af = AutoFix.insertionBefore(s.tokens.front, " { ")
+ .concat(AutoFix.insertionAfter(s.tokens.back, " } "));
+ af.name = "Wrap in braces";
+
+ addErrorMessage(loc, KEY, stmtKind ~ MESSAGE_POSTFIX, [af]);
+ }
+ else
+ {
+ addErrorMessage(loc, KEY, stmtKind ~ MESSAGE_POSTFIX);
+ }
+ }
+ }
+
+ override void visit(const(IfStatement) stmt)
+ {
+ auto s = stmt.thenStatement.statement;
+ this.test(stmt.thenStatement, s, "if");
+ if (stmt.elseStatement !is null)
+ {
+ auto e = stmt.elseStatement.statement;
+ this.test(stmt.elseStatement, e, "else");
+ }
+ }
+
+ override void visit(const(ForStatement) stmt)
+ {
+ auto s = stmt.declarationOrStatement;
+ if (s.statement !is null)
+ {
+ this.test(s, s, "for");
+ }
+ }
+
+ override void visit(const(ForeachStatement) stmt)
+ {
+ auto s = stmt.declarationOrStatement;
+ if (s.statement !is null)
+ {
+ this.test(s, s, "foreach");
+ }
+ }
+
+ override void visit(const(TryStatement) stmt)
+ {
+ auto s = stmt.declarationOrStatement;
+ if (s.statement !is null)
+ {
+ this.test(s, s, "try");
+ }
+
+ if (stmt.catches !is null)
+ {
+ foreach (const(Catch) ct; stmt.catches.catches)
+ {
+ this.test(ct, ct.declarationOrStatement, "catch");
+ }
+ if (stmt.catches.lastCatch !is null)
+ {
+ auto sncnd = stmt.catches.lastCatch.statementNoCaseNoDefault;
+ if (sncnd !is null)
+ {
+ this.test(stmt.catches.lastCatch, sncnd, "finally");
+ }
+ }
+ }
+ }
+
+ override void visit(const(WhileStatement) stmt)
+ {
+ auto s = stmt.declarationOrStatement;
+ if (s.statement !is null)
+ {
+ this.test(s, s, "while");
+ }
+ }
+
+ override void visit(const(DoStatement) stmt)
+ {
+ auto s = stmt.statementNoCaseNoDefault;
+ if (s !is null)
+ {
+ this.test(s, s, "do");
+ }
+ }
+
+ enum string KEY = "dscanner.style.always_curly";
+ enum string MESSAGE_POSTFIX = " must be follow by a BlockStatement aka. { }";
+}
+
+unittest
+{
+ import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
+ import dscanner.analysis.helpers : assertAnalyzerWarnings, assertAutoFix;
+ import std.stdio : stderr;
+
+ StaticAnalysisConfig sac = disabledConfig();
+ sac.always_curly_check = Check.enabled;
+
+ assertAnalyzerWarnings(q{
+ void testIf()
+ {
+ if(true) return; // [warn]: if must be follow by a BlockStatement aka. { }
+ }
+ }, sac);
+
+ assertAnalyzerWarnings(q{
+ void testIf()
+ {
+ if(true) return; /+
+ ^^^^^^^ [warn]: if must be follow by a BlockStatement aka. { } +/
+ }
+ }, sac);
+
+ assertAnalyzerWarnings(q{
+ void testIf()
+ {
+ for(int i = 0; i < 10; ++i) return; // [warn]: for must be follow by a BlockStatement aka. { }
+ }
+ }, sac);
+
+ assertAnalyzerWarnings(q{
+ void testIf()
+ {
+ foreach(it; 0 .. 10) return; // [warn]: foreach must be follow by a BlockStatement aka. { }
+ }
+ }, sac);
+
+ assertAnalyzerWarnings(q{
+ void testIf()
+ {
+ while(true) return; // [warn]: while must be follow by a BlockStatement aka. { }
+ }
+ }, sac);
+
+ assertAnalyzerWarnings(q{
+ void testIf()
+ {
+ do return; while(true); return; // [warn]: do must be follow by a BlockStatement aka. { }
+ }
+ }, sac);
+}
+
+unittest {
+ import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
+ import dscanner.analysis.helpers : assertAnalyzerWarnings, assertAutoFix;
+ import std.stdio : stderr;
+
+ StaticAnalysisConfig sac = disabledConfig();
+ sac.always_curly_check = Check.enabled;
+
+ assertAutoFix(q{
+ void test() {
+ if(true) return; // fix:0
+ }
+ }c, q{
+ void test() {
+ if(true) { return; } // fix:0
+ }
+ }c, sac);
+
+ assertAutoFix(q{
+ void test() {
+ foreach(_; 0 .. 10 ) return; // fix:0
+ }
+ }c, q{
+ void test() {
+ foreach(_; 0 .. 10 ) { return; } // fix:0
+ }
+ }c, sac);
+
+ assertAutoFix(q{
+ void test() {
+ for(int i = 0; i < 10; ++i) return; // fix:0
+ }
+ }c, q{
+ void test() {
+ for(int i = 0; i < 10; ++i) { return; } // fix:0
+ }
+ }c, sac);
+
+ assertAutoFix(q{
+ void test() {
+ do return; while(true) // fix:0
+ }
+ }c, q{
+ void test() {
+ do { return; } while(true) // fix:0
+ }
+ }c, sac);
+
+
+ stderr.writeln("Unittest for AlwaysCurly passed.");
+}
diff --git a/src/dscanner/analysis/config.d b/src/dscanner/analysis/config.d
index 4827259..1e94f56 100644
--- a/src/dscanner/analysis/config.d
+++ b/src/dscanner/analysis/config.d
@@ -167,6 +167,9 @@ struct StaticAnalysisConfig
@INI("Check for auto function without return statement")
string auto_function_check = Check.disabled;
+ @INI("Check that if|else|for|foreach|while|do|try|catch are always followed by a BlockStatement { }")
+ string always_curly_check = Check.disabled;
+
@INI("Check for sortedness of imports")
string imports_sortedness = Check.disabled;
diff --git a/src/dscanner/analysis/run.d b/src/dscanner/analysis/run.d
index dc1af0a..e9c69d5 100644
--- a/src/dscanner/analysis/run.d
+++ b/src/dscanner/analysis/run.d
@@ -73,6 +73,7 @@ import dscanner.analysis.final_attribute;
import dscanner.analysis.vcall_in_ctor;
import dscanner.analysis.useless_initializer;
import dscanner.analysis.allman;
+import dscanner.analysis.always_curly;
import dscanner.analysis.redundant_attributes;
import dscanner.analysis.has_public_example;
import dscanner.analysis.assert_without_msg;
@@ -917,6 +918,10 @@ private BaseAnalyzer[] getAnalyzersForModuleAndConfig(string fileName,
checks ~= new AllManCheck(fileName, tokens,
analysisConfig.allman_braces_check == Check.skipTests && !ut);
+ if (moduleName.shouldRun!AlwaysCurlyCheck(analysisConfig))
+ checks ~= new AlwaysCurlyCheck(fileName, tokens,
+ analysisConfig.always_curly_check == Check.skipTests && !ut);
+
if (moduleName.shouldRun!RedundantAttributesCheck(analysisConfig))
checks ~= new RedundantAttributesCheck(fileName, moduleScope,
analysisConfig.redundant_attributes_check == Check.skipTests && !ut);