Skip to content

Commit

Permalink
Use DMD in RedundantStorageClassCheck (#84)
Browse files Browse the repository at this point in the history
  • Loading branch information
Vladiwostok authored Jan 15, 2024
1 parent c3c7ee1 commit 820a5a8
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 51 deletions.
83 changes: 36 additions & 47 deletions src/dscanner/analysis/redundant_storage_class.d
Original file line number Diff line number Diff line change
Expand Up @@ -5,74 +5,68 @@

module dscanner.analysis.redundant_storage_class;

import std.stdio;
import std.string;
import dparse.ast;
import dparse.lexer;
import dscanner.analysis.base;
import dscanner.analysis.helpers;
import dsymbol.scope_ : Scope;

/**
* Checks for redundant storage classes such immutable and __gshared, static and __gshared
*/
final class RedundantStorageClassCheck : BaseAnalyzer
extern (C++) class RedundantStorageClassCheck(AST) : BaseAnalyzerDmd
{
alias visit = BaseAnalyzer.visit;
enum string REDUNDANT_VARIABLE_ATTRIBUTES = "Variable declaration for `%s` has redundant attributes (%-(`%s`%|, %)).";
alias visit = BaseAnalyzerDmd.visit;
mixin AnalyzerInfo!"redundant_storage_classes";

this(string fileName, bool skipTests = false)
{
super(fileName, null, skipTests);
}
private enum KEY = "dscanner.unnecessary.duplicate_attribute";
private enum string REDUNDANT_VARIABLE_ATTRIBUTES = "Variable declaration for `%s` has redundant attributes (%-(`%s`%|, %)).";

override void visit(const Declaration node)
extern (D) this(string fileName, bool skipTests = false)
{
checkAttributes(node);
node.accept(this);
super(fileName, skipTests);
}

void checkAttributes(const Declaration node)
override void visit(AST.VarDeclaration varDecl)
{
if (node.variableDeclaration !is null && node.attributes !is null)
checkVariableDeclaration(node.variableDeclaration, node.attributes);
import dmd.astenums : STC;

if (varDecl.storage_class & STC.immutable_ && varDecl.storage_class & STC.shared_)
addErrorFor(varDecl, "immutable", "shared");

if (varDecl.storage_class & STC.immutable_ && varDecl.storage_class & STC.gshared)
addErrorFor(varDecl, "immutable", "__gshared");

if (varDecl.storage_class & STC.static_ && varDecl.storage_class & STC.gshared)
addErrorFor(varDecl, "static", "__gshared");
}

void checkVariableDeclaration(const VariableDeclaration vd, const Attribute[] attributes)
extern (D) private void addErrorFor(AST.VarDeclaration varDecl, string attr1, string attr2)
{
import std.algorithm.comparison : among;
import std.algorithm.searching: all;

string[] globalAttributes;
foreach (attrib; attributes)
{
if (attrib.attribute.type.among(tok!"shared", tok!"static", tok!"__gshared", tok!"immutable"))
globalAttributes ~= attrib.attribute.type.str;
}
if (globalAttributes.length > 1)
{
if (globalAttributes.length == 2 && (
globalAttributes.all!(a => a.among("shared", "static")) ||
globalAttributes.all!(a => a.among("static", "immutable"))
))
return;
auto t = vd.declarators[0].name;
string message = REDUNDANT_VARIABLE_ATTRIBUTES.format(t.text, globalAttributes);
addErrorMessage(t.line, t.column, "dscanner.unnecessary.duplicate_attribute", message);
}
auto lineNum = cast(ulong) varDecl.loc.linnum;
auto charNum = cast(ulong) varDecl.loc.charnum;
auto varName = varDecl.ident.toString();
auto errorMsg = REDUNDANT_VARIABLE_ATTRIBUTES.format(varName, [
attr1, attr2
]);
addErrorMessage(lineNum, charNum, KEY, errorMsg);
}
}

unittest
{
import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
import dscanner.analysis.helpers : assertAnalyzerWarningsDMD;
import std.stdio : stderr;
import std.format : format;

StaticAnalysisConfig sac = disabledConfig();
sac.redundant_storage_classes = Check.enabled;

enum string erorMsg = "Variable declaration for `%s` has redundant attributes (%-(`%s`%|, %)).";
auto immutableSharedMsg = erorMsg.format("a", ["immutable", "shared"]);
auto immutableGSharedMsg = erorMsg.format("a", ["immutable", "__gshared"]);
auto staticGSharedMsg = erorMsg.format("a", ["static", "__gshared"]);

// https://github.com/dlang-community/D-Scanner/issues/438
assertAnalyzerWarnings(q{
assertAnalyzerWarningsDMD(q{
immutable int a;

immutable shared int a; // [warn]: %s
Expand All @@ -91,13 +85,8 @@ unittest
enum int a;
extern(C++) immutable int a;
immutable int function(immutable int, shared int) a;
}c.format(
RedundantStorageClassCheck.REDUNDANT_VARIABLE_ATTRIBUTES.format("a", ["immutable", "shared"]),
RedundantStorageClassCheck.REDUNDANT_VARIABLE_ATTRIBUTES.format("a", ["shared", "immutable"]),
RedundantStorageClassCheck.REDUNDANT_VARIABLE_ATTRIBUTES.format("a", ["immutable", "__gshared"]),
RedundantStorageClassCheck.REDUNDANT_VARIABLE_ATTRIBUTES.format("a", ["__gshared", "immutable"]),
RedundantStorageClassCheck.REDUNDANT_VARIABLE_ATTRIBUTES.format("a", ["__gshared", "static"]),
), sac);
}c.format(immutableSharedMsg, immutableSharedMsg, immutableGSharedMsg,
immutableGSharedMsg, staticGSharedMsg), sac);

stderr.writeln("Unittest for RedundantStorageClassCheck passed.");
}
10 changes: 6 additions & 4 deletions src/dscanner/analysis/run.d
Original file line number Diff line number Diff line change
Expand Up @@ -545,10 +545,6 @@ MessageSet analyze(string fileName, const Module m, const StaticAnalysisConfig a
checks ~= new IfConstraintsIndentCheck(fileName, tokens,
analysisConfig.if_constraints_indent == Check.skipTests && !ut);

if (moduleName.shouldRun!RedundantStorageClassCheck(analysisConfig))
checks ~= new RedundantStorageClassCheck(fileName,
analysisConfig.redundant_storage_classes == Check.skipTests && !ut);

if (moduleName.shouldRun!UnusedResultChecker(analysisConfig))
checks ~= new UnusedResultChecker(fileName, moduleScope,
analysisConfig.unused_result == Check.skipTests && !ut);
Expand Down Expand Up @@ -695,6 +691,12 @@ MessageSet analyzeDmd(string fileName, ASTCodegen.Module m, const char[] moduleN
config.asm_style_check == Check.skipTests && !ut
);

if (moduleName.shouldRunDmd!(RedundantStorageClassCheck!ASTCodegen)(config))
visitors ~= new RedundantStorageClassCheck!ASTCodegen(
fileName,
config.redundant_storage_classes == Check.skipTests && !ut
);

foreach (visitor; visitors)
{
m.accept(visitor);
Expand Down

0 comments on commit 820a5a8

Please sign in to comment.