From 4301f6a67e23caee71124d2b7382a5e406523f03 Mon Sep 17 00:00:00 2001 From: Vladiwostok <55026261+Vladiwostok@users.noreply.github.com> Date: Thu, 22 Feb 2024 11:40:22 +0200 Subject: [PATCH] Delete DuplicateAttributeCheck (#79) --- .../dscanner.duplicate-attribute-check.dd | 2 + src/dscanner/analysis/duplicate_attribute.d | 266 ------------------ src/dscanner/analysis/run.d | 5 - 3 files changed, 2 insertions(+), 271 deletions(-) create mode 100644 changelog/dscanner.duplicate-attribute-check.dd delete mode 100644 src/dscanner/analysis/duplicate_attribute.d diff --git a/changelog/dscanner.duplicate-attribute-check.dd b/changelog/dscanner.duplicate-attribute-check.dd new file mode 100644 index 00000000..d2d045d0 --- /dev/null +++ b/changelog/dscanner.duplicate-attribute-check.dd @@ -0,0 +1,2 @@ +Remove the check for duplicate attributes (@property, @safe, @trusted, @system, pure, nothrow). +This check is no longer necessary since having duplicated attributes is now a compiler error. diff --git a/src/dscanner/analysis/duplicate_attribute.d b/src/dscanner/analysis/duplicate_attribute.d deleted file mode 100644 index 7e6ec446..00000000 --- a/src/dscanner/analysis/duplicate_attribute.d +++ /dev/null @@ -1,266 +0,0 @@ -// Copyright (c) 2014, Matthew Brennan Jones -// 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.duplicate_attribute; - -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 duplicate attributes such as @property, @safe, - * @trusted, @system, pure, and nothrow - */ -final class DuplicateAttributeCheck : BaseAnalyzer -{ - alias visit = BaseAnalyzer.visit; - - mixin AnalyzerInfo!"duplicate_attribute"; - - this(BaseAnalyzerArguments args) - { - super(args); - } - - override void visit(const Declaration node) - { - checkAttributes(node); - node.accept(this); - } - - void checkAttributes(const Declaration node) - { - bool hasProperty; - bool hasSafe; - bool hasTrusted; - bool hasSystem; - bool hasPure; - bool hasNoThrow; - - // Check the attributes - foreach (attribute; node.attributes) - { - const(Token)[] tokens; - string attributeName = getAttributeName(attribute, tokens); - if (!attributeName || !tokens.length) - return; - - // Check for the attributes - checkDuplicateAttribute(attributeName, "property", tokens, hasProperty); - checkDuplicateAttribute(attributeName, "safe", tokens, hasSafe); - checkDuplicateAttribute(attributeName, "trusted", tokens, hasTrusted); - checkDuplicateAttribute(attributeName, "system", tokens, hasSystem); - checkDuplicateAttribute(attributeName, "pure", tokens, hasPure); - checkDuplicateAttribute(attributeName, "nothrow", tokens, hasNoThrow); - } - - // Just return if missing function nodes - if (!node.functionDeclaration || !node.functionDeclaration.memberFunctionAttributes) - return; - - // Check the functions - foreach (memberFunctionAttribute; node.functionDeclaration.memberFunctionAttributes) - { - const(Token)[] tokens; - string attributeName = getAttributeName(memberFunctionAttribute, tokens); - if (!attributeName || !tokens.length) - return; - - // Check for the attributes - checkDuplicateAttribute(attributeName, "property", tokens, hasProperty); - checkDuplicateAttribute(attributeName, "safe", tokens, hasSafe); - checkDuplicateAttribute(attributeName, "trusted", tokens, hasTrusted); - checkDuplicateAttribute(attributeName, "system", tokens, hasSystem); - checkDuplicateAttribute(attributeName, "pure", tokens, hasPure); - checkDuplicateAttribute(attributeName, "nothrow", tokens, hasNoThrow); - } - } - - void checkDuplicateAttribute(const string attributeName, - const string attributeDesired, const(Token)[] tokens, ref bool hasAttribute) - { - // Just return if not an attribute - if (attributeName != attributeDesired) - return; - - // Already has that attribute - if (hasAttribute) - { - string message = "Attribute '%s' is duplicated.".format(attributeName); - addErrorMessage(tokens, "dscanner.unnecessary.duplicate_attribute", message, - [AutoFix.replacement(tokens, "", "Remove second attribute " ~ attributeName)]); - } - - // Mark it as having that attribute - hasAttribute = true; - } - - string getAttributeName(const Attribute attribute, ref const(Token)[] outTokens) - { - // Get the name from the attribute identifier - if (attribute && attribute.atAttribute && attribute.atAttribute.identifier !is Token.init - && attribute.atAttribute.identifier.text - && attribute.atAttribute.identifier.text.length) - { - auto token = attribute.atAttribute.identifier; - outTokens = attribute.atAttribute.tokens; - return token.text; - } - - // Get the attribute from the storage class token - if (attribute && attribute.attribute.type != tok!"") - { - outTokens = attribute.tokens; - return attribute.attribute.type.str; - } - - return null; - } - - string getAttributeName(const MemberFunctionAttribute memberFunctionAttribute, - ref const(Token)[] outTokens) - { - // Get the name from the tokenType - if (memberFunctionAttribute && memberFunctionAttribute.tokenType !is IdType.init - && memberFunctionAttribute.tokenType.str - && memberFunctionAttribute.tokenType.str.length) - { - outTokens = memberFunctionAttribute.tokens; - return memberFunctionAttribute.tokenType.str; - } - - // Get the name from the attribute identifier - if (memberFunctionAttribute && memberFunctionAttribute.atAttribute - && memberFunctionAttribute.atAttribute.identifier !is Token.init - && memberFunctionAttribute.atAttribute.identifier.type == tok!"identifier" - && memberFunctionAttribute.atAttribute.identifier.text - && memberFunctionAttribute.atAttribute.identifier.text.length) - { - auto iden = memberFunctionAttribute.atAttribute.identifier; - outTokens = memberFunctionAttribute.atAttribute.tokens; - return iden.text; - } - - return null; - } -} - -unittest -{ - import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig; - - StaticAnalysisConfig sac = disabledConfig(); - sac.duplicate_attribute = Check.enabled; - assertAnalyzerWarnings(q{ - class ExampleAttributes - { - @property @safe bool xxx() // ok - { - return false; - } - - // Duplicate before - @property @property bool aaa() /+ - ^^^^^^^^^ [warn]: Attribute 'property' is duplicated. +/ - { - return false; - } - - // Duplicate after - bool bbb() @safe @safe /+ - ^^^^^ [warn]: Attribute 'safe' is duplicated. +/ - { - return false; - } - - // Duplicate before and after - @system bool ccc() @system /+ - ^^^^^^^ [warn]: Attribute 'system' is duplicated. +/ - { - return false; - } - - // Duplicate before and after - @trusted bool ddd() @trusted /+ - ^^^^^^^^ [warn]: Attribute 'trusted' is duplicated. +/ - { - return false; - } - } - - class ExamplePureNoThrow - { - pure nothrow bool aaa() // ok - { - return false; - } - - pure pure bool bbb() /+ - ^^^^ [warn]: Attribute 'pure' is duplicated. +/ - { - return false; - } - - bool ccc() pure pure /+ - ^^^^ [warn]: Attribute 'pure' is duplicated. +/ - { - return false; - } - - nothrow nothrow bool ddd() /+ - ^^^^^^^ [warn]: Attribute 'nothrow' is duplicated. +/ - { - return false; - } - - bool eee() nothrow nothrow /+ - ^^^^^^^ [warn]: Attribute 'nothrow' is duplicated. +/ - { - return false; - } - } - }c, sac); - - - assertAutoFix(q{ - class ExampleAttributes - { - @property @property bool aaa() {} // fix - bool bbb() @safe @safe {} // fix - @system bool ccc() @system {} // fix - @trusted bool ddd() @trusted {} // fix - } - - class ExamplePureNoThrow - { - pure pure bool bbb() {} // fix - bool ccc() pure pure {} // fix - nothrow nothrow bool ddd() {} // fix - bool eee() nothrow nothrow {} // fix - } - }c, q{ - class ExampleAttributes - { - @property bool aaa() {} // fix - bool bbb() @safe {} // fix - @system bool ccc() {} // fix - @trusted bool ddd() {} // fix - } - - class ExamplePureNoThrow - { - pure bool bbb() {} // fix - bool ccc() pure {} // fix - nothrow bool ddd() {} // fix - bool eee() nothrow {} // fix - } - }c, sac); - - stderr.writeln("Unittest for DuplicateAttributeCheck passed."); -} diff --git a/src/dscanner/analysis/run.d b/src/dscanner/analysis/run.d index 5e46a8b9..e3fc866e 100644 --- a/src/dscanner/analysis/run.d +++ b/src/dscanner/analysis/run.d @@ -40,7 +40,6 @@ import dscanner.analysis.constructors; import dscanner.analysis.unused_variable; import dscanner.analysis.unused_label; import dscanner.analysis.unused_parameter; -import dscanner.analysis.duplicate_attribute; import dscanner.analysis.opequals_without_tohash; import dscanner.analysis.length_subtraction; import dscanner.analysis.builtin_property_names; @@ -840,10 +839,6 @@ private BaseAnalyzer[] getAnalyzersForModuleAndConfig(string fileName, checks ~= new UnmodifiedFinder(args.setSkipTests( analysisConfig.could_be_immutable_check == Check.skipTests && !ut)); - if (moduleName.shouldRun!DuplicateAttributeCheck(analysisConfig)) - checks ~= new DuplicateAttributeCheck(args.setSkipTests( - analysisConfig.duplicate_attribute == Check.skipTests && !ut)); - if (moduleName.shouldRun!FunctionAttributeCheck(analysisConfig)) checks ~= new FunctionAttributeCheck(args.setSkipTests( analysisConfig.function_attribute_check == Check.skipTests && !ut));