Add matterlint support for deny cluster .... attribute ... (#30147)

* Add support for matterlint deny and explicitly deny wearcount on GeneralDiagnostics

* Restyle

* A bit of documentation in comments

* Force global attributes in matter files always (since we will codegen based on them)

* Revert "Force global attributes in matter files always (since we will codegen based on them)"

This reverts commit 23b312e.

* Fix docs in lark file

* Clean whitespace

* Fix mypy

* Fix typing

* Can remove some lint restrictions - all clusters passes lint now, some bugs are old

.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

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

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__':

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,13 +218,22 @@ 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
 
     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:

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 {