diff --git a/README.md b/README.md index 5e2028e..c8f4828 100644 --- a/README.md +++ b/README.md @@ -57,6 +57,7 @@ given source files. * 'if' statements where the 'else' block is the same as the 'if' block. * Unused variables. * Unused parameters (check is skipped if function is marked "override") +* Duplicate attributes #### Wishlish * Assigning to foreach variables that are not "ref". diff --git a/analysis/duplicate_attribute.d b/analysis/duplicate_attribute.d new file mode 100644 index 0000000..556d319 --- /dev/null +++ b/analysis/duplicate_attribute.d @@ -0,0 +1,229 @@ +// 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 analysis.duplicate_attribute; + +import std.stdio; +import std.string; +import std.d.ast; +import std.d.lexer; +import analysis.base; +import analysis.helpers; + + +/** + * Checks for duplicate attributes such as @property, @safe, + * @trusted, @system, pure, and nothrow + */ +class DuplicateAttributeCheck : BaseAnalyzer +{ + alias visit = BaseAnalyzer.visit; + + this(string fileName) + { + super(fileName); + } + + override void visit(const Declaration node) + { + checkAttributes(node); + node.accept(this); + } + + void checkAttributes(const Declaration node) + { + bool hasProperty = false; + bool hasSafe = false; + bool hasTrusted = false; + bool hasSystem = false; + bool hasPure = false; + bool hasNoThrow = false; + + // Check the attributes + foreach (attribute; node.attributes) + { + size_t line, column; + string attributeName = getAttributeName(attribute, line, column); + if (!attributeName || line==0 || column==0) + return; + + // Check for the attributes + checkDuplicateAttribute(attributeName, "property", line, column, hasProperty); + checkDuplicateAttribute(attributeName, "safe", line, column, hasSafe); + checkDuplicateAttribute(attributeName, "trusted", line, column, hasTrusted); + checkDuplicateAttribute(attributeName, "system", line, column, hasSystem); + checkDuplicateAttribute(attributeName, "pure", line, column, hasPure); + checkDuplicateAttribute(attributeName, "nothrow", line, column, hasNoThrow); + } + + // Just return if missing function nodes + if (!node.functionDeclaration + || !node.functionDeclaration.memberFunctionAttributes) + return; + + // Check the functions + foreach (memberFunctionAttribute; node.functionDeclaration.memberFunctionAttributes) + { + size_t line, column; + string attributeName = getAttributeName(memberFunctionAttribute, line, column); + if (!attributeName || line==0 || column==0) + return; + + // Check for the attributes + checkDuplicateAttribute(attributeName, "property", line, column, hasProperty); + checkDuplicateAttribute(attributeName, "safe", line, column, hasSafe); + checkDuplicateAttribute(attributeName, "trusted", line, column, hasTrusted); + checkDuplicateAttribute(attributeName, "system", line, column, hasSystem); + checkDuplicateAttribute(attributeName, "pure", line, column, hasPure); + checkDuplicateAttribute(attributeName, "nothrow", line, column, hasNoThrow); + } + } + + void checkDuplicateAttribute(const string attributeName, const string attributeDesired, size_t line, size_t column, ref bool hasAttribute) + { + // Just return if not an attribute + if (attributeName != attributeDesired) + return; + + // Already has that attribute + if (hasAttribute) + { + string message = "The attribute '%s' is duplicated.".format(attributeName); + addErrorMessage(line, column, message); + } + + // Mark it as having that attribute + hasAttribute = true; + } + + string getAttributeName(const Attribute attribute, ref size_t line, ref size_t column) + { + // Get the name from the attribute identifier + if (attribute + && attribute.storageClass + && attribute.storageClass.atAttribute + && attribute.storageClass.atAttribute.identifier !is Token.init + && attribute.storageClass.atAttribute.identifier.text + && attribute.storageClass.atAttribute.identifier.text.length) + { + auto token = attribute.storageClass.atAttribute.identifier; + line = token.line; + column = token.column; + return token.text; + } + + // Get the attribute from the storage class token + if (attribute + && attribute.storageClass + && attribute.storageClass.token !is Token.init) + { + auto token = attribute.storageClass.token; + line = token.line; + column = token.column; + return token.type.str; + } + + return null; + } + + string getAttributeName(const MemberFunctionAttribute memberFunctionAttribute, ref size_t line, ref size_t column) + { + // Get the name from the tokenType + if (memberFunctionAttribute + && memberFunctionAttribute.tokenType !is IdType.init + && memberFunctionAttribute.tokenType.str + && memberFunctionAttribute.tokenType.str.length) + { + // FIXME: How do we get the line/column number? + 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; + line = iden.line; + column = iden.column; + return iden.text; + } + + return null; + } +} + +unittest +{ + assertAnalyzerWarnings(q{ + class ExampleAttributes + { + @property @safe bool xxx() // ok + { + return false; + } + + // Duplicate before + @property @property bool aaa() // [warn]: The attribute 'property' is duplicated. + { + return false; + } + + // Duplicate after + bool bbb() @safe @safe // [warn]: The attribute 'safe' is duplicated. + { + return false; + } + + // Duplicate before and after + @system bool ccc() @system // [warn]: The attribute 'system' is duplicated. + { + return false; + } + + // Duplicate before and after + @trusted bool ddd() @trusted // [warn]: The attribute 'trusted' is duplicated. + { + return false; + } + } + + class ExamplePureNoThrow + { + pure nothrow bool aaa() // ok + { + return false; + } + + pure pure bool bbb() // [warn]: The attribute 'pure' is duplicated. + { + return false; + } + + // FIXME: There is no way to get the line/column number of the attribute like this + bool ccc() pure pure // FIXME: [warn]: The attribute 'pure' is duplicated. + { + return false; + } + + nothrow nothrow bool ddd() // [warn]: The attribute 'nothrow' is duplicated. + { + return false; + } + + // FIXME: There is no way to get the line/column number of the attribute like this + bool eee() nothrow nothrow // FIXME: [warn]: The attribute 'nothrow' is duplicated. + { + return false; + } + } + }c, analysis.run.AnalyzerCheck.duplicate_attribute); + + stderr.writeln("Unittest for DuplicateAttributeCheck passed."); +} + diff --git a/analysis/helpers.d b/analysis/helpers.d index e4bbc0a..ae081c2 100644 --- a/analysis/helpers.d +++ b/analysis/helpers.d @@ -8,6 +8,7 @@ module analysis.helpers; import std.string; import std.traits; import std.d.ast; +import analysis.run; S between(S)(S value, S before, S after) diff --git a/analysis/run.d b/analysis/run.d index 8737cbc..88b1e55 100644 --- a/analysis/run.d +++ b/analysis/run.d @@ -22,6 +22,7 @@ import analysis.range; import analysis.ifelsesame; import analysis.constructors; import analysis.unused; +import analysis.duplicate_attribute; enum AnalyzerCheck : int { @@ -36,7 +37,8 @@ enum AnalyzerCheck : int if_else_same_check = 0x100, constructor_check = 0x200, unused_variable_check = 0x400, - all = 0x7FF + duplicate_attribute = 0x800, + all = 0xFFF } void messageFunction(string fileName, size_t line, size_t column, string message, @@ -104,6 +106,7 @@ string[] analyze(string fileName, ubyte[] code, AnalyzerCheck analyzers, bool st if (analyzers & AnalyzerCheck.if_else_same_check) checks ~= new IfElseSameCheck(fileName); if (analyzers & AnalyzerCheck.constructor_check) checks ~= new ConstructorCheck(fileName); if (analyzers & AnalyzerCheck.unused_variable_check) checks ~= new UnusedVariableCheck(fileName); + if (analyzers & AnalyzerCheck.duplicate_attribute) checks ~= new DuplicateAttributeCheck(fileName); foreach (check; checks) { diff --git a/std/allocator.d b/std/allocator.d index be494e9..e4aae8e 100644 --- a/std/allocator.d +++ b/std/allocator.d @@ -4337,7 +4337,7 @@ class CAllocator throw an exception if it does allow setting the alignment but an invalid value is passed. */ - @property bool alignment(uint) pure nothrow @property + @property bool alignment(uint) pure nothrow { return false; }