diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index b70ac09cb8ad5b..cc53e3ced79ac1 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -51,9 +51,6 @@ jobs: # https://github.com/project-chip/connectedhomeip/issues/19176 # https://github.com/project-chip/connectedhomeip/issues/19175 # https://github.com/project-chip/connectedhomeip/issues/19173 - # https://github.com/project-chip/connectedhomeip/issues/19169 - # https://github.com/project-chip/connectedhomeip/issues/22640 - if [ "$idl_file" = './examples/all-clusters-app/all-clusters-common/all-clusters-app.matter' ]; then continue; fi if [ "$idl_file" = './examples/log-source-app/log-source-common/log-source-app.matter' ]; then continue; fi if [ "$idl_file" = './examples/placeholder/linux/apps/app1/config.matter' ]; then continue; fi if [ "$idl_file" = './examples/placeholder/linux/apps/app2/config.matter' ]; then continue; fi diff --git a/scripts/py_matter_idl/matter_idl/lint/lint_rules_grammar.lark b/scripts/py_matter_idl/matter_idl/lint/lint_rules_grammar.lark index 90cfd6e8d441f7..63ffd17cd1c263 100644 --- a/scripts/py_matter_idl/matter_idl/lint/lint_rules_grammar.lark +++ b/scripts/py_matter_idl/matter_idl/lint/lint_rules_grammar.lark @@ -4,25 +4,33 @@ instruction: load_xml|all_endpoint_rule|specific_endpoint_rule load_xml: "load" ESCAPED_STRING ";" -all_endpoint_rule: "all" "endpoints" "{" required_global_attribute* "}" +all_endpoint_rule: "all" "endpoints" "{" (required_global_attribute|denylist_cluster_attribute)* "}" specific_endpoint_rule: "endpoint" integer "{" (required_server_cluster|rejected_server_cluster)* "}" required_global_attribute: "require" "global" "attribute" id "=" integer ";" -required_server_cluster: "require" "server" "cluster" (id|POSITIVE_INTEGER|HEX_INTEGER) ";" +required_server_cluster: "require" "server" "cluster" id_or_number ";" -rejected_server_cluster: "reject" "server" "cluster" (id|POSITIVE_INTEGER|HEX_INTEGER) ";" +rejected_server_cluster: "reject" "server" "cluster" id_or_number ";" + +// Allows deny like: +// deny cluster 234; // Deny an entire cluster +// deny cluster DescriptorCluster attribute 123; // attribute deny (mix name and number) +// deny cluster 11 attribute 22; // attribute deny +denylist_cluster_attribute: "deny" "cluster" id_or_number ["attribute" id_or_number] ";" integer: positive_integer | negative_integer +?id_or_number: (id|positive_integer) + positive_integer: POSITIVE_INTEGER | HEX_INTEGER negative_integer: "-" positive_integer id: ID -POSITIVE_INTEGER: /\d+/ -HEX_INTEGER: /0x[A-Fa-f0-9]+/ +POSITIVE_INTEGER: /\d+/ +HEX_INTEGER: /0x[A-Fa-f0-9]+/ ID: /[a-zA-Z_][a-zA-Z0-9_]*/ %import common.ESCAPED_STRING diff --git a/scripts/py_matter_idl/matter_idl/lint/lint_rules_parser.py b/scripts/py_matter_idl/matter_idl/lint/lint_rules_parser.py index 9499e360399230..09c2f270afe94c 100755 --- a/scripts/py_matter_idl/matter_idl/lint/lint_rules_parser.py +++ b/scripts/py_matter_idl/matter_idl/lint/lint_rules_parser.py @@ -11,15 +11,15 @@ from lark.visitors import Discard, Transformer, v_args try: - from .types import (AttributeRequirement, ClusterCommandRequirement, ClusterRequirement, ClusterValidationRule, - RequiredAttributesRule, RequiredCommandsRule) + from .types import (AttributeRequirement, ClusterAttributeDeny, ClusterCommandRequirement, ClusterRequirement, + ClusterValidationRule, RequiredAttributesRule, RequiredCommandsRule) except ImportError: import sys sys.path.append(os.path.join(os.path.abspath( os.path.dirname(__file__)), "..", "..")) - from matter_idl.lint.types import (AttributeRequirement, ClusterCommandRequirement, ClusterRequirement, ClusterValidationRule, - RequiredAttributesRule, RequiredCommandsRule) + from matter_idl.lint.types import (AttributeRequirement, ClusterAttributeDeny, ClusterCommandRequirement, ClusterRequirement, + ClusterValidationRule, RequiredAttributesRule, RequiredCommandsRule) class ElementNotFoundError(Exception): @@ -167,6 +167,9 @@ def GetLinterRules(self): def RequireAttribute(self, r: AttributeRequirement): self._required_attributes_rule.RequireAttribute(r) + def Deny(self, what: ClusterAttributeDeny): + self._required_attributes_rule.Deny(what) + def FindClusterCode(self, name: str) -> Optional[Tuple[str, int]]: if name not in self._cluster_codes: # Name may be a number. If this can be parsed as a number, accept it anyway @@ -281,9 +284,14 @@ def start(self, instructions): def instruction(self, instruction): return Discard - def all_endpoint_rule(self, attributes): - for attribute in attributes: - self.context.RequireAttribute(attribute) + def all_endpoint_rule(self, rules: List[Union[AttributeRequirement, ClusterAttributeDeny]]): + for rule in rules: + if type(rule) is AttributeRequirement: + self.context.RequireAttribute(rule) + elif type(rule) is ClusterAttributeDeny: + self.context.Deny(rule) + else: + raise Exception("Unkown endpoint requirement: %r" % rule) return Discard @@ -320,6 +328,10 @@ def required_server_cluster(self, id): def rejected_server_cluster(self, id): return ServerClusterRequirement(ClusterActionEnum.REJECT, id) + @v_args(inline=True) + def denylist_cluster_attribute(self, cluster_id, attribute_id): + return ClusterAttributeDeny(cluster_id, attribute_id) + class Parser: def __init__(self, parser, file_name: str): @@ -337,7 +349,7 @@ def CreateParser(file_name: str): Generates a parser that will process a ".matter" file into a IDL """ return Parser( - Lark.open('lint_rules_grammar.lark', rel_to=__file__, parser='lalr', propagate_positions=True), file_name=file_name) + Lark.open('lint_rules_grammar.lark', rel_to=__file__, parser='lalr', propagate_positions=True, maybe_placeholders=True), file_name=file_name) if __name__ == '__main__': diff --git a/scripts/py_matter_idl/matter_idl/lint/types.py b/scripts/py_matter_idl/matter_idl/lint/types.py index 1b148ac430c47a..d0764f1e044f14 100644 --- a/scripts/py_matter_idl/matter_idl/lint/types.py +++ b/scripts/py_matter_idl/matter_idl/lint/types.py @@ -14,7 +14,7 @@ from abc import ABC, abstractmethod from dataclasses import dataclass, field -from typing import List, MutableMapping, Optional +from typing import List, MutableMapping, Optional, Union from matter_idl.matter_idl_types import ClusterSide, Idl, ParseMetaData @@ -75,6 +75,12 @@ class AttributeRequirement: filter_cluster: Optional[int] = field(default=None) +@dataclass +class ClusterAttributeDeny: + cluster_id: Union[str, int] + attribute_id: Union[str, int] + + @dataclass class ClusterRequirement: endpoint_id: int @@ -202,6 +208,7 @@ class RequiredAttributesRule(ErrorAccumulatingRule): def __init__(self, name): super().__init__(name) self._mandatory_attributes: List[AttributeRequirement] = [] + self._deny_attributes: List[ClusterAttributeDeny] = [] def __repr__(self): result = "RequiredAttributesRule{\n" @@ -211,6 +218,11 @@ def __repr__(self): for attr in self._mandatory_attributes: result += " - %r\n" % attr + if self._deny_attributes: + result += " deny_attributes:\n" + for attr in self._deny_attributes: + result += " - %r\n" % attr + result += "}" return result @@ -218,6 +230,10 @@ def RequireAttribute(self, attr: AttributeRequirement): """Mark an attribute required""" self._mandatory_attributes.append(attr) + def Deny(self, what: ClusterAttributeDeny): + """Mark a cluster (or cluster/attribute) as denied""" + self._deny_attributes.append(what) + def _ServerClusterDefinition(self, name: str, location: Optional[LocationInFile]): """Finds the server cluster definition with the given name. @@ -275,7 +291,7 @@ def _LintImpl(self): attribute_codes.add(name_to_code_map[attr.name]) - # Linting codes now + # Linting required attributes for check in self._mandatory_attributes: if check.filter_cluster is not None and check.filter_cluster != cluster_definition.code: continue @@ -286,6 +302,27 @@ def _LintImpl(self): check.name, check.code), self._ParseLocation(cluster.parse_meta)) + # Lint rejected attributes + for check in self._deny_attributes: + if check.cluster_id not in [cluster_definition.name, cluster_definition.code]: + continue # different cluster + + if check.attribute_id is None: + self._AddLintError( + f"EP{endpoint.number}: cluster {cluster_definition.name}({cluster_definition.code}) is DENIED!", + self._ParseLocation(cluster.parse_meta)) + continue + + # figure out every attribute that may be denied + # We already know every attribute name and have codes + for attr in cluster.attributes: + if check.attribute_id in [attr.name, name_to_code_map[attr.name]]: + cluster_str = f"{cluster_definition.name}({cluster_definition.code})" + attribute_str = f"{attr.name}({name_to_code_map[attr.name]})" + self._AddLintError( + f"EP{endpoint.number}: attribute {cluster_str}::{attribute_str} is DENIED!", + self._ParseLocation(cluster.parse_meta)) + @dataclass class ClusterCommandRequirement: diff --git a/scripts/rules.matterlint b/scripts/rules.matterlint index e8fd7d07d469d9..a120c0c25b4e99 100644 --- a/scripts/rules.matterlint +++ b/scripts/rules.matterlint @@ -105,6 +105,16 @@ all endpoints { require global attribute featureMap = 65532; require global attribute clusterRevision = 65533; + + // Deny rules: + // deny cluster Foo; // denies entire cluster by name + // deny cluster 123; // denies entire cluster by id + // deny cluster Foo attribute bar; // denies specific attribute by name + // deny cluster Foo attribute 123; // denies specific attribute by id + // deny cluster 234 attribute 567; // Allows deny using ids only + + // (previously AverageWearCount, see #30002/#29285 in GitHub) + deny cluster GeneralDiagnostics attribute 9; } endpoint 0 {