diff --git a/zircon/system/host/fidl/include/fidl/linting_tree_callbacks.h b/zircon/system/host/fidl/include/fidl/linting_tree_callbacks.h index 4e3149c4d4f0224546023d8e85ee9025f6fdb2fc..e40b970750a6e9480ef5fcf4ca0264eacc2ecc82 100644 --- a/zircon/system/host/fidl/include/fidl/linting_tree_callbacks.h +++ b/zircon/system/host/fidl/include/fidl/linting_tree_callbacks.h @@ -2,8 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef ZIRCON_SYSTEM_HOST_FIDL_INCLUDE_FIDL_CALLBACK_TREE_VISITOR_H_ -#define ZIRCON_SYSTEM_HOST_FIDL_INCLUDE_FIDL_CALLBACK_TREE_VISITOR_H_ +#ifndef ZIRCON_SYSTEM_HOST_FIDL_INCLUDE_FIDL_LINTING_TREE_CALLBACKS_H_ +#define ZIRCON_SYSTEM_HOST_FIDL_INCLUDE_FIDL_LINTING_TREE_CALLBACKS_H_ #include <vector> @@ -40,6 +40,9 @@ public: void OnBitsDeclaration(fit::function<void(const raw::BitsDeclaration&)> callback) { bits_declaration_callbacks_.push_back(std::move(callback)); } + void OnBitsMember(fit::function<void(const raw::BitsMember&)> callback) { + bits_member_callbacks_.push_back(std::move(callback)); + } void OnConstDeclaration(fit::function<void(const raw::ConstDeclaration&)> callback) { const_declaration_callbacks_.push_back(std::move(callback)); } @@ -52,6 +55,15 @@ public: void OnInterfaceDeclaration(fit::function<void(const raw::InterfaceDeclaration&)> callback) { interface_declaration_callbacks_.push_back(std::move(callback)); } + void OnMethod(fit::function<void(const raw::InterfaceMethod&)> callback) { + method_callbacks_.push_back(std::move(callback)); + } + void OnEvent(fit::function<void(const raw::InterfaceMethod&)> callback) { + event_callbacks_.push_back(std::move(callback)); + } + void OnParameter(fit::function<void(const raw::Parameter&)> callback) { + parameter_callbacks_.push_back(std::move(callback)); + } void OnStructMember(fit::function<void(const raw::StructMember&)> callback) { struct_member_callbacks_.push_back(std::move(callback)); } @@ -93,10 +105,14 @@ private: std::vector<fit::function<void(const raw::File&)>> file_callbacks_; std::vector<fit::function<void(const raw::Using&)>> using_callbacks_; std::vector<fit::function<void(const raw::BitsDeclaration&)>> bits_declaration_callbacks_; + std::vector<fit::function<void(const raw::BitsMember&)>> bits_member_callbacks_; std::vector<fit::function<void(const raw::ConstDeclaration&)>> const_declaration_callbacks_; std::vector<fit::function<void(const raw::EnumDeclaration&)>> enum_declaration_callbacks_; std::vector<fit::function<void(const raw::EnumMember&)>> enum_member_callbacks_; std::vector<fit::function<void(const raw::InterfaceDeclaration&)>> interface_declaration_callbacks_; + std::vector<fit::function<void(const raw::InterfaceMethod&)>> method_callbacks_; + std::vector<fit::function<void(const raw::InterfaceMethod&)>> event_callbacks_; + std::vector<fit::function<void(const raw::Parameter&)>> parameter_callbacks_; std::vector<fit::function<void(const raw::StructMember&)>> struct_member_callbacks_; std::vector<fit::function<void(const raw::StructDeclaration&)>> struct_declaration_callbacks_; std::vector<fit::function<void(const raw::TableMember&)>> table_member_callbacks_; @@ -110,4 +126,4 @@ private: } // namespace linter } // namespace fidl -#endif // ZIRCON_SYSTEM_HOST_FIDL_INCLUDE_FIDL_CALLBACK_TREE_VISITOR_H_ +#endif // ZIRCON_SYSTEM_HOST_FIDL_INCLUDE_FIDL_LINTING_TREE_CALLBACKS_H_ diff --git a/zircon/system/host/fidl/lib/linter.cpp b/zircon/system/host/fidl/lib/linter.cpp index 4761561b3517c26bf2c414173afb1ba5fe958ff0..bd2e9107ade1817c2ec13aa43d8d5d9c36dc0c73 100644 --- a/zircon/system/host/fidl/lib/linter.cpp +++ b/zircon/system/host/fidl/lib/linter.cpp @@ -194,7 +194,7 @@ Linter::Linter() // (const raw::Using& element) { if (element.maybe_alias != nullptr) { - CHECK_CASE("Primitive alias", lower_snake, element.maybe_alias) + CHECK_CASE("primitive alias", lower_snake, element.maybe_alias) } }); @@ -207,7 +207,7 @@ Linter::Linter() check = invalid_case_for_constant] // (const raw::ConstDeclaration& element) { - CHECK_CASE("Constants", upper_snake, element.identifier) + CHECK_CASE("constants", upper_snake, element.identifier) }); callbacks_.OnEnumMember( @@ -218,6 +218,14 @@ Linter::Linter() CHECK_CASE("enum members", upper_snake, element.identifier) }); + callbacks_.OnBitsMember( + [& linter = *this, + check = invalid_case_for_constant] + // + (const raw::BitsMember& element) { + CHECK_CASE("bitfield members", upper_snake, element.identifier) + }); + auto& invalid_case_for_decl_name = DefineCheck( "invalid-case-for-decl-name", "${CHECK_SUBTYPE} must be named in UpperCamelCase"); @@ -227,7 +235,23 @@ Linter::Linter() check = invalid_case_for_decl_name] // (const raw::InterfaceDeclaration& element) { - CHECK_CASE("Protocols", upper_camel, element.identifier) + CHECK_CASE("protocols", upper_camel, element.identifier) + }); + + callbacks_.OnMethod( + [& linter = *this, + check = invalid_case_for_decl_name] + // + (const raw::InterfaceMethod& element) { + CHECK_CASE("methods", upper_camel, element.identifier) + }); + + callbacks_.OnEvent( + [& linter = *this, + check = invalid_case_for_decl_name] + // + (const raw::InterfaceMethod& element) { + CHECK_CASE("events", upper_camel, element.identifier) }); callbacks_.OnEnumDeclaration( @@ -235,7 +259,7 @@ Linter::Linter() check = invalid_case_for_decl_name] // (const raw::EnumDeclaration& element) { - CHECK_CASE("Enums", upper_camel, element.identifier) + CHECK_CASE("enums", upper_camel, element.identifier) }); callbacks_.OnBitsDeclaration( @@ -243,7 +267,7 @@ Linter::Linter() check = invalid_case_for_decl_name] // (const raw::BitsDeclaration& element) { - CHECK_CASE("Bitfields", upper_camel, element.identifier) + CHECK_CASE("bitfields", upper_camel, element.identifier) }); callbacks_.OnStructDeclaration( @@ -251,7 +275,7 @@ Linter::Linter() check = invalid_case_for_decl_name] // (const raw::StructDeclaration& element) { - CHECK_CASE("Structs", upper_camel, element.identifier) + CHECK_CASE("structs", upper_camel, element.identifier) }); callbacks_.OnTableDeclaration( @@ -259,7 +283,7 @@ Linter::Linter() check = invalid_case_for_decl_name] // (const raw::TableDeclaration& element) { - CHECK_CASE("Tables", upper_camel, element.identifier) + CHECK_CASE("tables", upper_camel, element.identifier) }); callbacks_.OnUnionDeclaration( @@ -267,7 +291,7 @@ Linter::Linter() check = invalid_case_for_decl_name] // (const raw::UnionDeclaration& element) { - CHECK_CASE("Unions", upper_camel, element.identifier) + CHECK_CASE("unions", upper_camel, element.identifier) }); callbacks_.OnXUnionDeclaration( @@ -275,7 +299,7 @@ Linter::Linter() check = invalid_case_for_decl_name] // (const raw::XUnionDeclaration& element) { - CHECK_CASE("XUnions", upper_camel, element.identifier) + CHECK_CASE("xunions", upper_camel, element.identifier) }); callbacks_.OnFile( @@ -325,14 +349,21 @@ Linter::Linter() auto& invalid_case_for_decl_member = DefineCheck( "invalid-case-for-decl-member", - "${CHECK_SUBTYPE} members must be named in lower_snake_case"); + "${CHECK_SUBTYPE} must be named in lower_snake_case"); + callbacks_.OnParameter( + [& linter = *this, + check = invalid_case_for_decl_member] + // + (const raw::Parameter& element) { + CHECK_CASE("parameters", lower_snake, element.identifier) + }); callbacks_.OnStructMember( [& linter = *this, check = invalid_case_for_decl_member] // (const raw::StructMember& element) { - CHECK_CASE("struct", lower_snake, element.identifier) + CHECK_CASE("struct members", lower_snake, element.identifier) }); callbacks_.OnTableMember( [& linter = *this, @@ -340,7 +371,7 @@ Linter::Linter() // (const raw::TableMember& element) { if (element.maybe_used != nullptr) { - CHECK_CASE("table", lower_snake, element.maybe_used->identifier) + CHECK_CASE("table members", lower_snake, element.maybe_used->identifier) } }); callbacks_.OnUnionMember( @@ -348,14 +379,14 @@ Linter::Linter() check = invalid_case_for_decl_member] // (const raw::UnionMember& element) { - CHECK_CASE("union", lower_snake, element.identifier) + CHECK_CASE("union members", lower_snake, element.identifier) }); callbacks_.OnXUnionMember( [& linter = *this, check = invalid_case_for_decl_member] // (const raw::XUnionMember& element) { - CHECK_CASE("xunion", lower_snake, element.identifier) + CHECK_CASE("xunion members", lower_snake, element.identifier) }); } diff --git a/zircon/system/host/fidl/lib/linting_tree_callbacks.cpp b/zircon/system/host/fidl/lib/linting_tree_callbacks.cpp index 135479328515953d77f4de0f286c90d0728ad65e..9afa818fe12aa2026a93ca3ee6b4dfc4ab9c75b6 100644 --- a/zircon/system/host/fidl/lib/linting_tree_callbacks.cpp +++ b/zircon/system/host/fidl/lib/linting_tree_callbacks.cpp @@ -42,6 +42,12 @@ LintingTreeCallbacks::LintingTreeCallbacks() { } DeclarationOrderTreeVisitor::OnBitsDeclaration(element); } + void OnBitsMember(std::unique_ptr<raw::BitsMember> const& element) override { + for (auto& callback : callbacks_.bits_member_callbacks_) { + callback(*element.get()); + } + DeclarationOrderTreeVisitor::OnBitsMember(element); + } void OnEnumMember(std::unique_ptr<raw::EnumMember> const& element) override { for (auto& callback : callbacks_.enum_member_callbacks_) { callback(*element.get()); @@ -60,6 +66,24 @@ LintingTreeCallbacks::LintingTreeCallbacks() { } DeclarationOrderTreeVisitor::OnInterfaceDeclaration(element); } + void OnInterfaceMethod(std::unique_ptr<raw::InterfaceMethod> const& element) override { + if (element->maybe_request != nullptr) { + for (auto& callback : callbacks_.method_callbacks_) { + callback(*element.get()); + } + } else { + for (auto& callback : callbacks_.event_callbacks_) { + callback(*element.get()); + } + } + DeclarationOrderTreeVisitor::OnInterfaceMethod(element); + } + void OnParameter(std::unique_ptr<raw::Parameter> const& element) override { + for (auto& callback : callbacks_.parameter_callbacks_) { + callback(*element.get()); + } + DeclarationOrderTreeVisitor::OnParameter(element); + } void OnStructMember(std::unique_ptr<raw::StructMember> const& element) override { for (auto& callback : callbacks_.struct_member_callbacks_) { callback(*element.get()); diff --git a/zircon/system/utest/fidl-compiler/lint_findings_tests.cpp b/zircon/system/utest/fidl-compiler/lint_findings_tests.cpp index 4f8973c2a03e26681777a00b1787650539e788d7..a7df4e7c2587e3bec9b14cddf702bcf834dd8c83 100644 --- a/zircon/system/utest/fidl-compiler/lint_findings_tests.cpp +++ b/zircon/system/utest/fidl-compiler/lint_findings_tests.cpp @@ -131,24 +131,11 @@ private: Substitutions substitutions_; }; -bool invalid_case_for_constant_checking_bitfield_member_please_implement_me() { - if (true) - return true; // disabled pending feature implementation - BEGIN_TEST; - - // Implement the check, then - // UNCOMMENT BITFIELD CONSTANT CHECK TESTS IN: - // invalid_case_for_constant() - // And remove this test function. - - END_TEST; -} - bool invalid_case_for_constant() { BEGIN_TEST; std::map<std::string, std::string> named_templates = { - {"Constants", R"FIDL( + {"constants", R"FIDL( library fidl.a; const uint64 ${TEST} = 1234; @@ -160,13 +147,13 @@ enum Int8Enum : int8 { ${TEST} = -1; }; )FIDL"}, - // {"Bitfield members", R"FIDL( // CHECK NOT YET IMPLEMENTED! - // library fidl.a; + {"bitfield members", R"FIDL( +library fidl.a; - // bits Uint32Bitfield : uint32 { - // ${TEST} = 0x00000004; - // }; - // )FIDL"}, +bits Uint32Bitfield : uint32 { + ${TEST} = 0x00000004; +}; +)FIDL"}, }; for (auto const& named_template : named_templates) { @@ -196,47 +183,61 @@ bool invalid_case_for_decl_name() { BEGIN_TEST; std::map<std::string, std::string> named_templates = { - {"Protocols", R"FIDL( + {"protocols", R"FIDL( library fidl.a; protocol ${TEST} {}; )FIDL"}, - {"Enums", R"FIDL( + {"methods", R"FIDL( +library fidl.a; + +protocol TestProtocol { + ${TEST}(); +}; +)FIDL"}, + {"events", R"FIDL( +library fidl.a; + +protocol TestProtocol { + -> ${TEST}(); +}; +)FIDL"}, + {"enums", R"FIDL( library fidl.a; enum ${TEST} : int8 { SOME_CONST = -1; }; )FIDL"}, - {"Bitfields", R"FIDL( + {"bitfields", R"FIDL( library fidl.a; bits ${TEST} : uint32 { SOME_BIT = 0x00000004; }; )FIDL"}, - {"Structs", R"FIDL( + {"structs", R"FIDL( library fidl.a; struct ${TEST} { string decl_member; }; )FIDL"}, - {"Tables", R"FIDL( + {"tables", R"FIDL( library fidl.a; table ${TEST} { 1: string decl_member; }; )FIDL"}, - {"Unions", R"FIDL( + {"unions", R"FIDL( library fidl.a; union ${TEST} { string decl_member; }; )FIDL"}, - {"XUnions", R"FIDL( + {"xunions", R"FIDL( library fidl.a; xunion ${TEST} { @@ -263,56 +264,39 @@ xunion ${TEST} { END_TEST; } -bool invalid_case_for_decl_member_checking_method_please_implement_me() { - if (true) - return true; // disabled pending feature implementation - BEGIN_TEST; - - // Implement the check and then add test templates to - // invalid_case_for_decl_member() - // And remove this test function. - - END_TEST; -} - -bool invalid_case_for_decl_member_checking_parameter_please_implement_me() { - if (true) - return true; // disabled pending feature implementation - BEGIN_TEST; - - // Implement the check and then add test templates to - // invalid_case_for_decl_member() - // And remove this test function. - - END_TEST; -} - bool invalid_case_for_decl_member() { BEGIN_TEST; std::map<std::string, std::string> named_templates = { - {"struct", R"FIDL( + {"parameters", R"FIDL( +library fidl.a; + +protocol TestProtocol { + TestMethod(string ${TEST}); +}; +)FIDL"}, + {"struct members", R"FIDL( library fidl.a; struct DeclName { string ${TEST}; }; )FIDL"}, - {"table", R"FIDL( + {"table members", R"FIDL( library fidl.a; table DeclName { 1: string ${TEST}; }; )FIDL"}, - {"union", R"FIDL( + {"union members", R"FIDL( library fidl.a; union DeclName { string ${TEST}; }; )FIDL"}, - {"xunion", R"FIDL( + {"xunion members", R"FIDL( library fidl.a; xunion DeclName { @@ -324,7 +308,7 @@ xunion DeclName { for (auto const& named_template : named_templates) { LintTest test; test.check_id("invalid-case-for-decl-member") - .message(named_template.first + " members must be named in lower_snake_case") + .message(named_template.first + " must be named in lower_snake_case") .source_template(named_template.second); test.substitute("TEST", "agent_request_count"); @@ -1934,10 +1918,7 @@ RUN_TEST(inconsistent_type_for_recurring_library_concept_please_implement_me) RUN_TEST(incorrect_line_indent_please_implement_me) RUN_TEST(incorrect_spacing_between_declarations_please_implement_me) RUN_TEST(invalid_case_for_constant) -RUN_TEST(invalid_case_for_constant_checking_bitfield_member_please_implement_me) // TO MERGE RUN_TEST(invalid_case_for_decl_member) -RUN_TEST(invalid_case_for_decl_member_checking_method_please_implement_me) // TO MERGE -RUN_TEST(invalid_case_for_decl_member_checking_parameter_please_implement_me) // TO MERGE RUN_TEST(invalid_case_for_decl_name) RUN_TEST(invalid_case_for_primitive_alias) RUN_TEST(invalid_copyright_for_platform_source_library_please_implement_me)