[fidl-lint] Remove repeater checks
Repeater checks attempt to enforce the aspect of the FIDL style guide
that says to avoid repeating a component from the parent context's
name (unless it serves to disambiguate).
This check has never been enabled due to false positives - given the
amount of time this feature has existed but not been touched,
we opt to remove this feature rather than continue maintaining it
(for example, updating it to support anonymous layouts). This is
done by simplifying the context stack from a deque<Context> into
a stack<string> representing the layout kinds (the deque -> stack
is an independent change to improve readability).
Test: fx ninja -C out/default host_x64/fidl-compiler &&
./out/default/host_x64/fidl-compiler
--gtest_filter='LintFindingsTests.*'
Change-Id: I26d5c869e5484b0928e7306bcba58b17febadeb4
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/560443
Reviewed-by: Ian McKellar <ianloic@google.com>
Commit-Queue: Felix Zhu <fcz@google.com>
diff --git a/tools/fidl/fidlc/include/fidl/linter.h b/tools/fidl/fidlc/include/fidl/linter.h
index 3a46b27..3e66496 100644
--- a/tools/fidl/fidlc/include/fidl/linter.h
+++ b/tools/fidl/fidlc/include/fidl/linter.h
@@ -6,9 +6,9 @@
#define TOOLS_FIDL_FIDLC_INCLUDE_FIDL_LINTER_H_
#include <array>
-#include <deque>
#include <regex>
#include <set>
+#include <stack>
#include <utility>
#include <fidl/check_def.h>
@@ -80,76 +80,6 @@
fit::function<std::string(std::string)> convert;
};
- // Holds information about a nesting context in a FIDL file, for checks
- // that must compare information about the context with information about
- // a nested entity. The outer-most context is the FIDL file itself
- // (including the file's declared library name). Contexts nested in a
- // file's context include type definitions with nested entities, such as
- // enum, bits, struct, table, and union.
- class Context {
- public:
- struct RepeatsContextNames;
-
- Context(std::string type, std::string id, CheckDef context_check)
- : type_(std::move(type)), id_(std::move(id)), context_check_(std::move(context_check)) {}
-
- // Enables move construction and assignment
- Context(Context&& rhs) = default;
- Context& operator=(Context&&) = default;
-
- // no copy or assign (move-only or pass by reference)
- Context(const Context&) = delete;
- Context& operator=(const Context&) = delete;
-
- std::string type() const { return type_; }
-
- std::string id() const { return id_; }
-
- // A |vector| of information about potential violations of the FIDL rubric
- // rule that prohibits repeating names from the outer type or library.
- // Exceptions to this rule cannot be determined until all nested identifiers
- // are reviewed, so this vector saves the required information until that
- // time.
- std::vector<RepeatsContextNames>& name_repeaters() { return name_repeaters_; }
-
- const std::set<std::string>& words() {
- if (words_.empty()) {
- auto words = utils::id_to_words(id_);
- words_.insert(words.begin(), words.end());
- }
- return words_;
- }
-
- const CheckDef& context_check() const { return context_check_; }
-
- template <typename... Args>
- void AddRepeatsContextNames(Args&&... args) {
- name_repeaters_.emplace_back(args...);
- }
-
- // Stores minimum information needed to construct a |Finding| if a
- // nested identifier repeats names from one of its contexts.
- // Determination is deferred until all nested identifiers are evaluated
- // because some cases of repeated names are allowed if the repeated
- // names help differentiate two identifiers that represent different
- // parts of the concept represented by the context identifier.
- struct RepeatsContextNames {
- RepeatsContextNames(std::string a_type, SourceSpan a_span, std::set<std::string> a_repeats)
- : type(std::move(a_type)), span(a_span), repeats(std::move(a_repeats)) {}
-
- const std::string type;
- const SourceSpan span;
- const std::set<std::string> repeats;
- };
-
- private:
- std::string type_;
- std::string id_;
- std::set<std::string> words_;
- CheckDef context_check_;
- std::vector<RepeatsContextNames> name_repeaters_;
- };
-
const std::set<std::string>& permitted_library_prefixes() const;
std::string kPermittedLibraryPrefixesas_string() const;
@@ -167,9 +97,6 @@
Substitutions substitutions = {}, std::string suggestion_template = "",
std::string replacement_template = "");
- const Finding* AddRepeatedNameFinding(const Context& context,
- const Context::RepeatsContextNames& name_repeater);
-
// Initialization and checks at the start of a new file. The |Linter|
// can be called multiple times with many different files.
void NewFile(const raw::File& element);
@@ -179,15 +106,7 @@
const std::unique_ptr<raw::Identifier>& identifier,
const CheckDef& check_def, const CaseType& case_type);
- // CheckRepeatedName() does not add Finding objects immediately. It checks for
- // potential violations, but must wait until ExitContext() so the potential
- // violation can be compared to its peers.
- void CheckRepeatedName(const std::string& type, const std::unique_ptr<raw::Identifier>& id);
-
- template <typename... Args>
- void EnterContext(Args&&... args) {
- context_stack_.emplace_front(args...);
- }
+ void EnterContext(const std::string& type) { type_stack_.push(type); }
// Pops the context stack. If any contained types repeat names from the
// context, this function compares the nested identifiers with each other.
@@ -214,7 +133,6 @@
// (https://google.github.io/styleguide/cppguide.html#Static_and_Global_Variables)
const CheckDef kLibraryNameDepthCheck;
const CheckDef kLibraryNameComponentCheck;
- const CheckDef kRepeatsLibraryNameCheck;
const CheckDef kLibraryPrefixCheck;
const CheckDef kInvalidCopyrightCheck;
@@ -228,7 +146,16 @@
const std::set<std::string> kPermittedLibraryPrefixes;
const std::set<std::string> kStopWords;
- std::deque<Context> context_stack_;
+ // Stores the stack of type contexts (roughly analogous to the NamingContext
+ // in flat/name.h, but storing the type rather than name), so that it is
+ // possible to determine the parent/containing type from within an On* visitor.
+ // For example, the `context_stack_` when calling OnOrdinaledMember for member
+ // `foo` in:
+ //
+ // type Data = struct { inner union { 1: foo uint8; }; };
+ //
+ // would be: ["struct", "union"].
+ std::stack<std::string> type_stack_;
size_t line_comments_checked_ = 0;
@@ -308,12 +235,10 @@
// populated only for the duration of the Visit(), to lint a given FIDL
// file.
//
- // Some lint checks can only be assessed after processing more of the FIDL
- // (for example, "name-repeats-enclosing-type-name" checks cannot be added
- // until all members of the enclosing type are available). Other checks on
- // the same members will be added as they are encountered. Since |Finding|
- // objects are collected out of source order, an ordered collection
- // ensures |Finding| objects are in sorted order.
+ // Some lint checks can only be assessed after processing more of the FIDL.
+ // Other checks on the same members will be added as they are encountered.
+ // Since |Finding| objects are collected out of source order, an ordered
+ // collection ensures |Finding| objects are in sorted order.
//
// |Finding| objects must be returned in source order (filename, starting
// character, and ending character).
diff --git a/tools/fidl/fidlc/lib/linter.cc b/tools/fidl/fidlc/lib/linter.cc
index d8dadb1..bc1bde3 100644
--- a/tools/fidl/fidlc/lib/linter.cc
+++ b/tools/fidl/fidlc/lib/linter.cc
@@ -226,8 +226,7 @@
}
}
}
- EnterContext("library", NameLibrary(element.library_decl->path->components),
- kRepeatsLibraryNameCheck);
+ EnterContext("library");
}
const Finding* Linter::CheckCase(const std::string& type,
@@ -246,40 +245,6 @@
return nullptr;
}
-void Linter::CheckRepeatedName(const std::string& type,
- const std::unique_ptr<raw::Identifier>& identifier) {
- std::string id = to_string(identifier);
- auto split_id = utils::id_to_words(id, kStopWords);
- std::set<std::string> words;
- words.insert(split_id.begin(), split_id.end());
- for (auto& context : context_stack_) {
- std::set<std::string> repeats;
- std::set_intersection(words.begin(), words.end(), context.words().begin(),
- context.words().end(), std::inserter(repeats, repeats.begin()));
- if (!repeats.empty()) {
- context.AddRepeatsContextNames(type, identifier->span(), repeats);
- }
- }
-}
-
-const Finding* Linter::AddRepeatedNameFinding(const Context& context,
- const Context::RepeatsContextNames& name_repeater) {
- std::string repeated_names;
- for (const auto& repeat : name_repeater.repeats) {
- if (!repeated_names.empty()) {
- repeated_names.append(", ");
- }
- repeated_names.append(repeat);
- }
- return AddFinding(name_repeater.span, context.context_check(),
- {
- {"TYPE", name_repeater.type},
- {"REPEATED_NAMES", repeated_names},
- {"CONTEXT_TYPE", context.type()},
- {"CONTEXT_ID", context.id()},
- });
-}
-
std::string Linter::GetCopyrightSuggestion() {
auto copyright_block = kCopyrightBlock;
if (!copyright_date_.empty()) {
@@ -329,43 +294,7 @@
good_copyright_lines_found_ >= kCopyrightLines.size();
}
-void Linter::ExitContext() {
- Context context = std::move(context_stack_.front());
- context_stack_.pop_front();
-
- // Check the |RepeatsContextNames| objects in context.name_repeaters(), and
- // produce Finding objects for any identifier that is not allowed to repeat
- // a name from this |Context|.
- //
- // This check addresses the FIDL Rubric rule:
- //
- // Member names must not repeat names from the enclosing type (or
- // library) unless the member name is ambiguous without a name from
- // the enclosing type...
- //
- // ...a type DeviceToRoom--that associates a smart device with the room
- // it's located in--may need to have members device_id and room_name,
- // because id and name are ambiguous; they could refer to either the
- // device or the room.
- auto& repeaters = context.name_repeaters();
- for (size_t i = 1; i < repeaters.size(); i++) {
- std::set<std::string> diffs;
- std::set_difference(repeaters[i - 1].repeats.begin(), repeaters[i - 1].repeats.end(),
- repeaters[i].repeats.begin(), repeaters[i].repeats.end(),
- std::inserter(diffs, diffs.begin()));
- if (!diffs.empty()) {
- // If there are any differences, we have to assume they may be
- // disambiguating, which is allowed.
- return;
- }
- }
- // If multiple name repeaters in a given context all repeat the same thing,
- // then it's obvious they don't disambiguate anything, so add Findings for
- // each violator.
- for (auto& repeater : repeaters) {
- AddRepeatedNameFinding(context, repeater);
- }
-}
+void Linter::ExitContext() { type_stack_.pop(); }
Linter::Linter()
: kLibraryNameDepthCheck(DefineCheck("too-many-nested-libraries",
@@ -374,10 +303,6 @@
DefineCheck("disallowed-library-name-component",
"Library names must not contain the following components: common, service, "
"util, base, f<letter>l, zx<word>")),
- kRepeatsLibraryNameCheck(
- DefineCheck("name-repeats-library-name",
- "${TYPE} names (${REPEATED_NAMES}) must not repeat names from the "
- "library '${CONTEXT_ID}'")),
kLibraryPrefixCheck(DefineCheck("wrong-prefix-for-platform-source-library",
"FIDL library name is not currently allowed")),
kInvalidCopyrightCheck(
@@ -508,10 +433,6 @@
DefineCheck("invalid-case-for-decl-member", "${TYPE} must be named in lower_snake_case");
auto modifiers_order = DefineCheck(
"modifier-order", "Strictness modifier on ${TYPE} must always precede the resource modifier");
- auto name_repeats_enclosing_type_name =
- DefineCheck("name-repeats-enclosing-type-name",
- "${TYPE} names (${REPEATED_NAMES}) must not repeat names from the "
- "enclosing ${CONTEXT_TYPE} '${CONTEXT_ID}'");
auto todo_should_not_be_doc_comment =
DefineCheck("todo-should-not-be-doc-comment",
"TODO comment should use a non-flow-through comment marker");
@@ -590,11 +511,6 @@
});
// clang-format on
- callbacks_.OnAliasDeclaration([&linter = *this]
- //
- (const raw::AliasDeclaration& element) {
- linter.CheckRepeatedName("type alias", element.alias);
- });
callbacks_.OnUsing([&linter = *this,
case_check = DefineCheck("invalid-case-for-using-alias",
"Using aliases must be named in lower_snake_case"),
@@ -604,7 +520,6 @@
if (element.maybe_alias != nullptr) {
linter.CheckCase("using alias", element.maybe_alias, case_check,
case_type);
- linter.CheckRepeatedName("using alias", element.maybe_alias);
}
});
@@ -613,7 +528,6 @@
//
(const raw::ConstDeclaration& element) {
linter.CheckCase("constants", element.identifier, case_check, case_type);
- linter.CheckRepeatedName("constant", element.identifier);
linter.in_const_declaration_ = true;
});
@@ -623,21 +537,20 @@
(const raw::ConstDeclaration& element) { linter.in_const_declaration_ = false; });
callbacks_.OnProtocolDeclaration(
- [&linter = *this, context_check = name_repeats_enclosing_type_name,
+ [&linter = *this,
name_contains_service_check = DefineCheck("protocol-name-includes-service",
"Protocols must not include the name 'service.'")]
//
(const raw::ProtocolDeclaration& element) {
linter.CheckCase("protocols", element.identifier, linter.invalid_case_for_decl_name(),
linter.decl_case_type_for_style());
- linter.CheckRepeatedName("protocol", element.identifier);
for (const auto& word : utils::id_to_words(to_string(element.identifier))) {
if (word == "service") {
linter.AddFinding(element.identifier, name_contains_service_check);
break;
}
}
- linter.EnterContext("protocol", to_string(element.identifier), context_check);
+ linter.EnterContext("protocol");
});
callbacks_.OnMethod([&linter = *this]
//
@@ -645,7 +558,6 @@
linter.CheckCase("methods", element.identifier,
linter.invalid_case_for_decl_name(),
linter.decl_case_type_for_style());
- linter.CheckRepeatedName("method", element.identifier);
});
callbacks_.OnEvent(
[&linter = *this, event_check = DefineCheck("event-names-must-start-with-on",
@@ -671,7 +583,6 @@
},
"change '${IDENTIFIER}' to '${REPLACEMENT}'", "${REPLACEMENT}");
}
- linter.CheckRepeatedName("event", element.identifier);
});
callbacks_.OnParameter(
[&linter = *this, case_check = invalid_case_for_decl_member, &case_type = lower_snake_]
@@ -705,76 +616,69 @@
}
});
- callbacks_.OnBitsDeclaration(
- [&linter = *this, context_check = name_repeats_enclosing_type_name]
- //
- (const raw::BitsDeclaration& element) {
- linter.CheckCase("bitfields", element.identifier, linter.invalid_case_for_decl_name(),
- linter.decl_case_type_for_style());
- linter.CheckRepeatedName("bitfield", element.identifier);
- linter.EnterContext("bitfield", to_string(element.identifier), context_check);
- });
+ callbacks_.OnBitsDeclaration([&linter = *this]
+ //
+ (const raw::BitsDeclaration& element) {
+ linter.CheckCase("bitfields", element.identifier,
+ linter.invalid_case_for_decl_name(),
+ linter.decl_case_type_for_style());
+ linter.EnterContext("bitfield");
+ });
callbacks_.OnBitsMember(
[&linter = *this, case_check = invalid_case_for_constant, &case_type = upper_snake_]
//
(const raw::BitsMember& element) {
linter.CheckCase("bitfield members", element.identifier, case_check, case_type);
- linter.CheckRepeatedName("bitfield member", element.identifier);
});
callbacks_.OnExitBitsDeclaration([&linter = *this]
//
(const raw::BitsDeclaration& element) { linter.ExitContext(); });
- callbacks_.OnEnumDeclaration(
- [&linter = *this, context_check = name_repeats_enclosing_type_name]
- //
- (const raw::EnumDeclaration& element) {
- linter.CheckCase("enums", element.identifier, linter.invalid_case_for_decl_name(),
- linter.decl_case_type_for_style());
- linter.CheckRepeatedName("enum", element.identifier);
- linter.EnterContext("enum", to_string(element.identifier), context_check);
- });
+ callbacks_.OnEnumDeclaration([&linter = *this]
+ //
+ (const raw::EnumDeclaration& element) {
+ linter.CheckCase("enums", element.identifier,
+ linter.invalid_case_for_decl_name(),
+ linter.decl_case_type_for_style());
+ linter.EnterContext("enum");
+ });
callbacks_.OnEnumMember(
[&linter = *this, case_check = invalid_case_for_constant, &case_type = upper_snake_]
//
(const raw::EnumMember& element) {
linter.CheckCase("enum members", element.identifier, case_check, case_type);
- linter.CheckRepeatedName("enum member", element.identifier);
});
callbacks_.OnExitEnumDeclaration([&linter = *this]
//
(const raw::EnumDeclaration& element) { linter.ExitContext(); });
- callbacks_.OnStructDeclaration(
- [&linter = *this, context_check = name_repeats_enclosing_type_name]
- //
- (const raw::StructDeclaration& element) {
- linter.CheckCase("structs", element.identifier, linter.invalid_case_for_decl_name(),
- linter.decl_case_type_for_style());
- linter.CheckRepeatedName("struct", element.identifier);
- linter.EnterContext("struct", to_string(element.identifier), context_check);
- });
+ callbacks_.OnStructDeclaration([&linter = *this]
+ //
+ (const raw::StructDeclaration& element) {
+ linter.CheckCase("structs", element.identifier,
+ linter.invalid_case_for_decl_name(),
+ linter.decl_case_type_for_style());
+ linter.EnterContext("struct");
+ });
callbacks_.OnStructMember(
[&linter = *this, case_check = invalid_case_for_decl_member, &case_type = lower_snake_]
//
(const raw::StructMember& element) {
linter.CheckCase("struct members", element.identifier, case_check, case_type);
- linter.CheckRepeatedName("struct member", element.identifier);
});
callbacks_.OnExitStructDeclaration(
[&linter = *this]
//
(const raw::StructDeclaration& element) { linter.ExitContext(); });
- callbacks_.OnTableDeclaration(
- [&linter = *this, context_check = name_repeats_enclosing_type_name]
- //
- (const raw::TableDeclaration& element) {
- linter.CheckCase("tables", element.identifier, linter.invalid_case_for_decl_name(),
- linter.decl_case_type_for_style());
- linter.CheckRepeatedName("table", element.identifier);
- linter.EnterContext("table", to_string(element.identifier), context_check);
- });
+ callbacks_.OnTableDeclaration([&linter = *this]
+ //
+ (const raw::TableDeclaration& element) {
+ linter.CheckCase("tables", element.identifier,
+ linter.invalid_case_for_decl_name(),
+ linter.decl_case_type_for_style());
+ linter.EnterContext("table");
+ });
callbacks_.OnTableMember(
[&linter = *this, case_check = invalid_case_for_decl_member, &case_type = lower_snake_]
//
@@ -782,22 +686,20 @@
if (element.maybe_used == nullptr)
return;
linter.CheckCase("table members", element.maybe_used->identifier, case_check, case_type);
- linter.CheckRepeatedName("table member", element.maybe_used->identifier);
});
callbacks_.OnExitTableDeclaration(
[&linter = *this]
//
(const raw::TableDeclaration& element) { linter.ExitContext(); });
- callbacks_.OnUnionDeclaration(
- [&linter = *this, context_check = name_repeats_enclosing_type_name]
- //
- (const raw::UnionDeclaration& element) {
- linter.CheckCase("unions", element.identifier, linter.invalid_case_for_decl_name(),
- linter.decl_case_type_for_style());
- linter.CheckRepeatedName("union", element.identifier);
- linter.EnterContext("union", to_string(element.identifier), context_check);
- });
+ callbacks_.OnUnionDeclaration([&linter = *this]
+ //
+ (const raw::UnionDeclaration& element) {
+ linter.CheckCase("unions", element.identifier,
+ linter.invalid_case_for_decl_name(),
+ linter.decl_case_type_for_style());
+ linter.EnterContext("union");
+ });
callbacks_.OnUnionMember(
[&linter = *this, case_check = invalid_case_for_decl_member, &case_type = lower_snake_]
//
@@ -805,7 +707,6 @@
if (element.maybe_used == nullptr)
return;
linter.CheckCase("union members", element.maybe_used->identifier, case_check, case_type);
- linter.CheckRepeatedName("union member", element.maybe_used->identifier);
});
callbacks_.OnExitUnionDeclaration(
[&linter = *this]
@@ -857,7 +758,7 @@
}
});
callbacks_.OnTypeDecl(
- [&linter = *this, context_check = name_repeats_enclosing_type_name]
+ [&linter = *this]
//
(const raw::TypeDecl& element) {
std::string context_type;
@@ -888,15 +789,14 @@
linter.CheckCase(context_type + "s", element.identifier,
linter.invalid_case_for_decl_name(), linter.decl_case_type_for_style());
- linter.CheckRepeatedName(context_type, element.identifier);
- linter.EnterContext(context_type, to_string(element.identifier), context_check);
+ linter.EnterContext(context_type);
});
callbacks_.OnLayout(
[&linter = *this, explict_flexible_modifier_check = explict_flexible_modifier,
modifiers_order_check = modifiers_order]
//
(const raw::Layout& element) {
- std::string parent_type = linter.context_stack_.front().type();
+ std::string parent_type = linter.type_stack_.top();
// The "parent_type" for type declarations is set in the OnTypeDecl callback. This callback
// is not invoked in the case of protocol request/response parameter lists, resulting a
// "protocol" naming context instead. However, since protocol request/response parameter
@@ -936,31 +836,27 @@
(const raw::OrdinaledLayoutMember& element) {
if (element.reserved)
return;
- std::string parent_type = linter.context_stack_.front().type();
+ std::string parent_type = linter.type_stack_.top();
linter.CheckCase(parent_type + " members", element.identifier, case_check, case_type);
- linter.CheckRepeatedName(parent_type + " member", element.identifier);
});
callbacks_.OnStructLayoutMember(
[&linter = *this, case_check = invalid_case_for_decl_member, &case_type = lower_snake_]
//
(const raw::StructLayoutMember& element) {
- std::string parent_type = linter.context_stack_.front().type();
+ std::string parent_type = linter.type_stack_.top();
if (parent_type == "protocol") {
linter.CheckCase("parameters", element.identifier, case_check, case_type);
- linter.CheckRepeatedName("parameters member", element.identifier);
return;
}
linter.CheckCase("struct members", element.identifier, case_check, case_type);
- linter.CheckRepeatedName("struct member", element.identifier);
});
callbacks_.OnValueLayoutMember(
[&linter = *this, case_check = invalid_case_for_constant, &case_type = upper_snake_]
//
(const raw::ValueLayoutMember& element) {
- std::string parent_type = linter.context_stack_.front().type();
+ std::string parent_type = linter.type_stack_.top();
linter.CheckCase(parent_type + " members", element.identifier, case_check, case_type);
- linter.CheckRepeatedName(parent_type + " member", element.identifier);
});
callbacks_.OnExitTypeDecl([&linter = *this]
//
diff --git a/tools/fidl/fidlc/linter/main.cc b/tools/fidl/fidlc/linter/main.cc
index 8cc31c8..2d036af 100644
--- a/tools/fidl/fidlc/linter/main.cc
+++ b/tools/fidl/fidlc/linter/main.cc
@@ -126,12 +126,6 @@
// The following checks can be opted-in via command line option "included-checks",
// but are otherwise disabled for the reasons described in the comments:
- // The name-repeats-* checks are very noisy, and sometimes produce
- // unexpected findings. Rules are being refined, but for now, these
- // are suppressed.
- excluded_and_disabled_checks.insert("name-repeats-library-name");
- excluded_and_disabled_checks.insert("name-repeats-enclosing-type-name");
-
// This check does currently highlight some potential issues with
// formatting and with 2-slash comments that will be converted to
// 3-slash Doc-Comments, but the rule cannot currently check 3-slash
diff --git a/zircon/system/utest/fidl-compiler/lint_findings_tests.cc b/zircon/system/utest/fidl-compiler/lint_findings_tests.cc
index 4a621c6..8768a386 100644
--- a/zircon/system/utest/fidl-compiler/lint_findings_tests.cc
+++ b/zircon/system/utest/fidl-compiler/lint_findings_tests.cc
@@ -499,76 +499,6 @@
}
}
-TEST(LintFindingsTests, ConstantRepeatsEnclosingTypeName) {
- std::map<std::string, std::string> named_templates = {
- {"enum", R"FIDL(
-library fidl.repeater;
-
-enum ConstantContainer : int8 {
- ${TEST} = -1;
-};
-)FIDL"},
- {"bitfield", R"FIDL(
-library fidl.repeater;
-
-bits ConstantContainer : uint32 {
- ${TEST} = 0x00000004;
-};
-)FIDL"},
- };
-
- for (auto const& named_template : named_templates) {
- LintTest test;
- test.check_id("name-repeats-enclosing-type-name").source_template(named_template.second);
-
- test.substitute("TEST", "SOME_VALUE");
- ASSERT_NO_FINDINGS(test);
-
- test.substitute("TEST", "SOME_CONSTANT")
- .message(named_template.first +
- " member names (constant) must not repeat names from the enclosing " +
- named_template.first + " 'ConstantContainer'");
- ASSERT_FINDINGS(test);
- }
-}
-
-TEST(LintFindingsTests, ConstantRepeatsLibraryName) {
- std::map<std::string, std::string> named_templates = {
- {"constant", R"FIDL(
-library fidl.repeater;
-
-const uint64 ${TEST} = 1234;
-)FIDL"},
- {"enum member", R"FIDL(
-library fidl.repeater;
-
-enum Int8Enum : int8 {
- ${TEST} = -1;
-};
-)FIDL"},
- {"bitfield member", R"FIDL(
-library fidl.repeater;
-
-bits Uint32Bitfield : uint32 {
- ${TEST} = 0x00000004;
-};
-)FIDL"},
- };
-
- for (auto const& named_template : named_templates) {
- LintTest test;
- test.check_id("name-repeats-library-name").source_template(named_template.second);
-
- test.substitute("TEST", "SOME_CONST");
- ASSERT_NO_FINDINGS(test);
-
- test.substitute("TEST", "LIBRARY_REPEATER")
- .message(named_template.first +
- " names (repeater) must not repeat names from the library 'fidl.repeater'");
- ASSERT_FINDINGS(test);
- }
-}
-
TEST(LintFindingsTests, ConstantShouldUseCommonPrefixSuffixPleaseImplementMe) {
if (true)
return; // disabled pending feature implementation
@@ -676,305 +606,6 @@
ASSERT_FINDINGS(test);
}
-TEST(LintFindingsTests, DeclMemberRepeatsEnclosingTypeName) {
- std::map<std::string, std::string> named_templates = {
- {"struct", R"FIDL(
-library fidl.repeater;
-
-struct DeclName {
- string:64 ${TEST};
-};
-)FIDL"},
- {"table", R"FIDL(
-library fidl.repeater;
-
-table DeclName {
- 1: string:64 ${TEST};
-};
-)FIDL"},
- {"union", R"FIDL(
-library fidl.repeater;
-
-union DeclName {
- 1: string:64 ${TEST};
-};
-)FIDL"},
- };
-
- for (auto const& named_template : named_templates) {
- LintTest test;
- test.check_id("name-repeats-enclosing-type-name").source_template(named_template.second);
-
- test.substitute("TEST", "some_member");
- ASSERT_NO_FINDINGS(test);
-
- test.substitute("TEST", "decl_member")
- .message(named_template.first +
- " member names (decl) must not repeat names from the enclosing " +
- named_template.first + " 'DeclName'");
- ASSERT_FINDINGS(test);
- }
-}
-
-TEST(LintFindingsTests, DeclMemberRepeatsEnclosingTypeNameButMayDisambiguate) {
- LintTest test;
- test.check_id("name-repeats-enclosing-type-name");
-
- test.source_template(R"FIDL(
-library fidl.repeater;
-
-struct SeasonToShirtAndPantMapEntry {
- string:64 season;
- string:64 shirt_type;
- string:64 pant_type;
-};
-)FIDL");
-
- ASSERT_NO_FINDINGS(test);
-
- test.source_template(R"FIDL(
-library fidl.repeater;
-
-struct SeasonToShirtAndPantMapEntry {
- string:64 season;
- string:64 shirt_and_pant_type;
- bool clashes;
-};
-)FIDL");
-
- ASSERT_NO_FINDINGS(test);
-
- test.source_template(R"FIDL(
-library fidl.repeater;
-
-struct SeasonToShirtAndPantMapEntry {
- string:64 season;
- string:64 shirt;
- string:64 shirt_for_season;
- bool clashes;
-};
-)FIDL");
-
- ASSERT_NO_FINDINGS(test);
-
- test.source_template(R"FIDL(
-library fidl.repeater;
-
-struct SeasonToShirtAndPantMapEntry {
- string:64 shirt_color;
- string:64 pant_color;
- bool clashes;
-};
-)FIDL");
-
- ASSERT_NO_FINDINGS(test);
-
- test.source_template(R"FIDL(
-library fidl.repeater;
-
-struct ShirtAndPantColor {
- string:64 shirt_color;
- string:64 pant_color;
- bool clashes;
-};
-)FIDL");
-
- ASSERT_NO_FINDINGS(test);
-
- test.source_template(R"FIDL(
-library fidl.repeater;
-
-struct NestedKeyValue {
- string:64 key_key;
- string:64 key_value;
- string:64 value_key;
- string:64 value_value;
-};
-)FIDL");
-
- ASSERT_NO_FINDINGS(test);
-
- test.source_template(R"FIDL(
-library fidl.repeater;
-
-struct ShirtAndPantSupplies {
- string:64 shirt_color;
- string:64 material;
- string:64 tag;
-};
-)FIDL")
- .AddFinding("name-repeats-enclosing-type-name",
- "struct member names (shirt) must not repeat names from the enclosing struct "
- "'ShirtAndPantSupplies'",
- "shirt_color");
-
- ASSERT_FINDINGS(test);
-
- test.source_template(R"FIDL(
-library fidl.repeater;
-
-struct ShirtAndPantSupplies {
- string:64 shirt_color;
- string:64 shirt_material;
- string:64 tag;
-};
-)FIDL")
- .AddFinding("name-repeats-enclosing-type-name",
- "struct member names (shirt) must not repeat names from the enclosing struct "
- "'ShirtAndPantSupplies'",
- "shirt_color")
- .AddFinding("name-repeats-enclosing-type-name",
- "struct member names (shirt) must not repeat names from the enclosing struct "
- "'ShirtAndPantSupplies'",
- "shirt_material");
-
- ASSERT_FINDINGS(test);
-
- test.source_template(R"FIDL(
-library fidl.repeater;
-
-struct ShirtAndPantSupplies {
- string:64 shirt_and_pant_color;
- string:64 material;
- string:64 shirt_and_pant_tag;
-};
-)FIDL")
- .AddFinding("name-repeats-enclosing-type-name",
- // repeated words are in lexicographical order; "and" is removed (a stop word)
- "struct member names (pant, shirt) must not repeat names from the enclosing "
- "struct 'ShirtAndPantSupplies'",
- "shirt_and_pant_color")
- .AddFinding("name-repeats-enclosing-type-name",
- // repeated words are in lexicographical order; "and" is removed (a stop word)
- "struct member names (pant, shirt) must not repeat names from the enclosing "
- "struct 'ShirtAndPantSupplies'",
- "shirt_and_pant_tag");
-
- ASSERT_FINDINGS(test);
-
- test.source_template(R"FIDL(
-library fidl.repeater;
-
-struct ShirtAndPantSupplies {
- string:64 shirt_and_pant_color;
- string:64 material;
- string:64 tag;
-};
-)FIDL")
- .AddFinding("name-repeats-enclosing-type-name",
- // repeated words are in lexicographical order; "and" is removed (a stop word)
- "struct member names (pant, shirt) must not repeat names from the enclosing "
- "struct 'ShirtAndPantSupplies'",
- "shirt_and_pant_color");
-
- ASSERT_FINDINGS(test);
-}
-
-TEST(LintFindingsTests, DeclMemberRepeatsLibraryName) {
- std::map<std::string, std::string> named_templates = {
- {"struct", R"FIDL(
-library fidl.repeater;
-
-struct DeclName {
- string:64 ${TEST};
-};
-)FIDL"},
- {"table", R"FIDL(
-library fidl.repeater;
-
-table DeclName {
- 1: string:64 ${TEST};
-};
-)FIDL"},
- {"union", R"FIDL(
-library fidl.repeater;
-
-union DeclName {
- 1: string:64 ${TEST};
-};
-)FIDL"},
- };
-
- for (auto const& named_template : named_templates) {
- LintTest test;
- test.check_id("name-repeats-library-name").source_template(named_template.second);
-
- test.substitute("TEST", "some_member");
- ASSERT_NO_FINDINGS(test);
-
- test.substitute("TEST", "library_repeater")
- .message(named_template.first +
- " member names (repeater) must not repeat names from the library "
- "'fidl.repeater'");
- ASSERT_FINDINGS(test);
- }
-}
-
-TEST(LintFindingsTests, DeclNameRepeatsLibraryName) {
- std::map<std::string, std::string> named_templates = {
- {"protocol", R"FIDL(
-library fidl.repeater;
-
-protocol ${TEST} {};
-)FIDL"},
- {"method", R"FIDL(
-library fidl.repeater;
-
-protocol TestProtocol {
- ${TEST}();
-};
-)FIDL"},
- {"enum", R"FIDL(
-library fidl.repeater;
-
-enum ${TEST} : int8 {
- SOME_CONST = -1;
-};
-)FIDL"},
- {"bitfield", R"FIDL(
-library fidl.repeater;
-
-bits ${TEST} : uint32 {
- SOME_BIT = 0x00000004;
-};
-)FIDL"},
- {"struct", R"FIDL(
-library fidl.repeater;
-
-struct ${TEST} {
- string:64 decl_member;
-};
-)FIDL"},
- {"table", R"FIDL(
-library fidl.repeater;
-
-table ${TEST} {
- 1: string:64 decl_member;
-};
-)FIDL"},
- {"union", R"FIDL(
-library fidl.repeater;
-
-union ${TEST} {
- 1: string:64 decl_member;
-};
-)FIDL"},
- };
-
- for (auto const& named_template : named_templates) {
- LintTest test;
- test.check_id("name-repeats-library-name").source_template(named_template.second);
-
- test.substitute("TEST", "UrlLoader");
- ASSERT_NO_FINDINGS(test);
-
- test.substitute("TEST", "LibraryRepeater")
- .message(named_template.first +
- " names (repeater) must not repeat names from the library 'fidl.repeater'");
- ASSERT_FINDINGS(test);
- }
-}
-
TEST(LintFindingsTests, DisallowedLibraryNameComponent) {
LintTest test;
test.check_id("disallowed-library-name-component")
@@ -1660,26 +1291,6 @@
ASSERT_FINDINGS(test);
}
-TEST(LintFindingsTests, MethodRepeatsEnclosingTypeName) {
- LintTest test;
- test.check_id("name-repeats-enclosing-type-name").source_template(R"FIDL(
-library fidl.repeater;
-
-protocol TestProtocol {
- ${TEST}();
-};
-)FIDL");
-
- test.substitute("TEST", "SomeMethod");
- ASSERT_NO_FINDINGS(test);
-
- test.substitute("TEST", "ProtocolMethod")
- .message(
- "method names (protocol) must not repeat names from the enclosing protocol "
- "'TestProtocol'");
- ASSERT_FINDINGS(test);
-}
-
TEST(LintFindingsTests, MethodReturnStatusMissingOkPleaseImplementMe) {
if (true)
return; // disabled pending feature implementation
@@ -2007,24 +1618,6 @@
ASSERT_FINDINGS(test);
}
-TEST(LintFindingsTests, TypeAliasRepeatsLibraryName) {
- LintTest test;
- test.check_id("name-repeats-library-name").source_template(R"FIDL(
-library fidl.repeater;
-
-alias ${TEST} = uint64;
-)FIDL");
-
- test.substitute("TEST", "some_alias");
- ASSERT_NO_FINDINGS(test);
-
- test.substitute("TEST", "library_repeater")
- .message(
- "type alias names (repeater) must not repeat names from the library "
- "'fidl.repeater'");
- ASSERT_FINDINGS(test);
-}
-
TEST(LintFindingsTests, ServiceHubPatternIsDiscouragedPleaseImplementMe) {
if (true)
return; // disabled pending feature implementation
