Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always curly #928

Merged
merged 1 commit into from
Sep 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
227 changes: 227 additions & 0 deletions src/dscanner/analysis/always_curly.d
Original file line number Diff line number Diff line change
@@ -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, " { ")
WebFreak001 marked this conversation as resolved.
Show resolved Hide resolved
WebFreak001 marked this conversation as resolved.
Show resolved Hide resolved
.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.");
}
3 changes: 3 additions & 0 deletions src/dscanner/analysis/config.d
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
5 changes: 5 additions & 0 deletions src/dscanner/analysis/run.d
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Loading