Do not exclude build directory from ruff check (#43572)

* Do not exclude build directory from ruff check

The `build` name is a very generic name which appears in our Python
modules and also is a directory for built artifacts. Instead of
excluding a generic name (which will disable ruff check in some part
of our code base) exclude specific paths.

* Apply ruff automatic fixes

* Fix logging warnings reported by ruff

* Fix Python boolean logic

* Restyled by autopep8

* Restyled by isort

Author: arkq

build/config/linux/pkg-config.py

@@ -107,10 +107,7 @@ def GetPkgConfigPrefixToStrip(options, args):
 def MatchesAnyRegexp(flag, list_of_regexps):
     """Returns true if the first argument matches any regular expression in the
     given list."""
-    for regexp in list_of_regexps:
-        if regexp.search(flag) is not None:
-            return True
-    return False
+    return any(regexp.search(flag) is not None for regexp in list_of_regexps)
 
 
 def RewritePath(path, strip_prefix, sysroot):

credentials/generate_revocation_set.py

@@ -358,7 +358,7 @@ def fetch_crl_from_url(url: str, timeout: int) -> x509.CertificateRevocationList
         log.debug(f"Fetched CRL: {r.content}")
         return x509.load_der_x509_crl(r.content)
     except Exception as e:
-        log.error('Failed to fetch a valid CRL', e)
+        log.error("Failed to fetch a valid CRL: %s", e)
 
 
 class DclClientInterface:
@@ -439,7 +439,7 @@ def get_paa_cert(self, initial_cert: x509.Certificate) -> Optional[x509.Certific
                     break
 
             except Exception as e:
-                log.error('Failed to get PAA certificate', e)
+                log.error("Failed to get PAA certificate: %s", e)
                 return None
             log.debug(f"issuer_name: {issuer_certificate.subject.rfc4514_string()}")
             issuer_name = issuer_certificate.issuer

pyproject.toml

@@ -12,14 +12,16 @@ known_first_party = "matter"
 line-length = 132
 target-version = "py311"
 exclude = [
-    ".environment",
-    ".git",
-    ".github",
     ".*",
-    "build",
     "out",
+    "scripts/py_matter_yamltests/build",
+    "scripts/py_matter_idl/build",
+    "src/python_testing/matter_testing_infrastructure/build",
+    # Exclude submodules since we do not maintain them
+    # TODO: Use exact paths once we will have a way to sync and validate them in CI
+    "examples/common/QRCode/repo",
+    "examples/common/m5stack-tft/repo",
     "third_party",
-    "examples/common/QRCode/",
     # TODO(#37698)
     "docs/development_controllers/matter-repl/*.ipynb",
 ]
@@ -30,7 +32,7 @@ reportMissingTypeStubs = "warning"
 [tool.ruff.lint]
 select = [
     # Standard errors and warnings
-    "E", "W", "F",
+    "E", "PLE", "W", "F",
     # Ensure that async is used properly
     "ASYNC",
     # Consistent commas
@@ -39,6 +41,8 @@ select = [
     "C4", "RET", "SIM",
     # Ensure that scripts are executable
     "EXE",
+    # Check for common exception issues
+    "RSE",
     # Check for common logging issues
     "LOG",
     # Check for common optimization issues

scripts/build/build/__init__.py

@@ -3,12 +3,13 @@
 import shutil
 import time
 from enum import Enum, auto
-from multiprocessing.pool import ThreadPool
-from typing import Optional, Sequence
+from typing import Sequence
 
 from builders.builder import BuilderOptions
 
-from .targets import BUILD_TARGETS, BuildTarget
+from .targets import BUILD_TARGETS
+
+log = logging.getLogger(__name__)
 
 
 class BuildSteps(Enum):
@@ -32,10 +33,10 @@ def time_builds(self, builders):
 
     def print_timing_report(self):
         total_time = self._total_end_time - self._total_start_time
-        logging.info("Build Time Summary:")
+        log.info("Build Time Summary:")
         for target, duration in self._build_times.items():
-            logging.info(f"  - {target}: {self._format_duration(duration)}")
-        logging.info(f"Total build time: {self._format_duration(total_time)}")
+            log.info(f"  - {target}: {self._format_duration(duration)}")
+        log.info(f"Total build time: {self._format_duration(total_time)}")
 
     def _format_duration(self, seconds):
         minutes = int(seconds // 60)
@@ -81,7 +82,7 @@ def SetupBuilders(self, targets: Sequence[str], options: BuilderOptions):
                     break
 
             if found_choice is None:
-                logging.error(f"Target '{target}' could not be found. Nothing executed for it")
+                log.error(f"Target '{target}' could not be found. Nothing executed for it")
                 continue
 
             parts = found_choice.StringIntoTargetParts(target)
@@ -109,7 +110,7 @@ def Generate(self):
         self.runner.StartCommandExecution()
 
         for builder in self.builders:
-            logging.info('Generating %s', builder.output_dir)
+            log.info('Generating %s', builder.output_dir)
             builder.generate()
 
         self.completed_steps.add(BuildSteps.GENERATED)
@@ -123,15 +124,15 @@ def Build(self):
 
     def CleanOutputDirectories(self):
         for builder in self.builders:
-            logging.warning('Cleaning %s', builder.output_dir)
+            log.warning('Cleaning %s', builder.output_dir)
             if os.path.exists(builder.output_dir):
                 shutil.rmtree(builder.output_dir)
 
         # any generated output was cleaned
         self.completed_steps.discard(BuildSteps.GENERATED)
 
     def CreateArtifactArchives(self, directory: str):
-        logging.info('Copying build artifacts to %s', directory)
+        log.info('Copying build artifacts to %s', directory)
         if not os.path.exists(directory):
             os.makedirs(directory)
         for builder in self.builders:
@@ -140,7 +141,7 @@ def CreateArtifactArchives(self, directory: str):
                 directory, f'{builder.identifier}.tar.gz'))
 
     def CopyArtifactsTo(self, path: str):
-        logging.info('Copying build artifacts to %s', path)
+        log.info('Copying build artifacts to %s', path)
         if not os.path.exists(path):
             os.makedirs(path)
 

scripts/build/build/target.py

@@ -47,6 +47,8 @@
 
 from builders.builder import BuilderOptions
 
+log = logging.getLogger(__name__)
+
 report_rejected_parts = True
 
 
@@ -83,14 +85,14 @@ def Accept(self, full_input: str):
             if self.except_if_re.search(full_input):
                 if report_rejected_parts:
                     # likely nothing will match when we get such an error
-                    logging.error(f"'{self.name}' does not support '{full_input}' due to rule EXCEPT IF '{self.except_if_re.pattern}'")
+                    log.error(f"'{self.name}' does not support '{full_input}' due to rule EXCEPT IF '{self.except_if_re.pattern}'")
                 return False
 
         if self.only_if_re:
             if not self.only_if_re.search(full_input):
                 if report_rejected_parts:
                     # likely nothing will match when we get such an error
-                    logging.error(f"'{self.name}' does not support '{full_input}' due to rule ONLY IF '{self.only_if_re.pattern}'")
+                    log.error(f"'{self.name}' does not support '{full_input}' due to rule ONLY IF '{self.only_if_re.pattern}'")
                 return False
 
         return True
@@ -140,6 +142,7 @@ def _HasVariantPrefix(value: str, prefix: str):
 
     if value.startswith(prefix + '-'):
         return value[len(prefix)+1:]
+    return None
 
 
 def _StringIntoParts(full_input: str, remaining_input: str, fixed_targets: List[List[TargetPart]], modifiers: List[TargetPart]):
@@ -224,10 +227,7 @@ def __init__(self, name, builder_class, **kwargs):
 
     def isUnifiedBuild(self, parts: List[TargetPart]):
         """Checks if the given parts combine into a unified build."""
-        for part in parts:
-            if part.build_arguments.get('unified', False):
-                return True
-        return False
+        return any(part.build_arguments.get('unified', False) for part in parts)
 
     def AppendFixedTargets(self, parts: List[TargetPart]):
         """Append a list of potential targets/variants.
@@ -283,7 +283,7 @@ def HumanString(self):
         result = self.name
         for fixed in self.fixed_targets:
             if len(fixed) > 1:
-                result += '-{' + ",".join(sorted(map(lambda x: x.name, fixed))) + '}'
+                result += '-{' + ",".join(sorted(x.name for x in fixed)) + '}'
             else:
                 result += '-' + fixed[0].name
 
@@ -411,9 +411,7 @@ def AllVariants(self) -> Iterable[str]:
 
         while True:
 
-            prefix = "-".join(map(
-                lambda p: self.fixed_targets[p[0]][p[1]].name, enumerate(fixed_indices)
-            ))
+            prefix = "-".join(self.fixed_targets[i][n].name for i, n in enumerate(fixed_indices))
 
             for n in range(len(self.modifiers) + 1):
                 for c in itertools.combinations(self.modifiers, n):
@@ -462,7 +460,7 @@ def Create(self, name: str, runner, repository_path: str, output_prefix: str,
         for part in parts:
             kargs.update(part.build_arguments)
 
-        logging.info("Preparing builder '%s'" % (name,))
+        log.info("Preparing builder '%s'" % (name,))
 
         builder = self.builder_class(repository_path, runner=runner, **kargs)
         builder.target = self

scripts/build/build/targets.py

@@ -13,6 +13,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 import argparse
+import contextlib
 import glob
 import os
 import platform
@@ -50,10 +51,8 @@ def run_command(command):
     if returncode != 0:
         # command_log is binary, so decoding as utf-8 might technically fail.  We don't want
         # to throw on that.
-        try:
+        with contextlib.suppress(Exception):
             print("Failure log: {}".format(command_log.decode()))
-        except Exception:
-            pass
 
     return returncode
 

scripts/build/build_darwin_framework.py

@@ -19,12 +19,13 @@
 import os
 import sys
 
-import build
 import click
 import coloredlogs
 from builders.builder import BuilderOptions
 from runner import PrintOnlyRunner, ShellRunner
 
+import build
+
 sys.path.append(os.path.abspath(os.path.dirname(__file__)))
 
 
@@ -38,10 +39,6 @@
 }
 
 
-def CommaSeparate(items) -> str:
-    return ', '.join([x for x in items])
-
-
 def ValidateRepoPath(context, parameter, value):
     """
     Validates that the given path looks like a valid chip repository checkout.
@@ -170,7 +167,7 @@ def main(context, log_level, verbose, target, enable_link_map_file, repo,
         ninja_jobs=ninja_jobs, runner=runner
     )
 
-    requested_targets = set([t.lower() for t in target])
+    requested_targets = {t.lower() for t in target}
     context.obj.SetupBuilders(targets=requested_targets, options=BuilderOptions(
         enable_link_map_file=enable_link_map_file,
         enable_flashbundle=enable_flashbundle,

scripts/build/build_examples.py

@@ -19,6 +19,8 @@
 
 from .builder import Builder, BuilderOutput
 
+log = logging.getLogger(__name__)
+
 
 class AndroidBoard(Enum):
     ARM = auto()
@@ -176,17 +178,17 @@ def _find_sdk_manager(self, for_purpose="validation"):
                 # Test environment - assume the path is valid
                 sdk_manager = path
                 checked_details.append(f"{path} (test environment - assumed valid)")
-                logging.info(f"Using SDK manager for {for_purpose} from test environment: {sdk_manager}")
+                log.info(f"Using SDK manager for {for_purpose} from test environment: {sdk_manager}")
                 break
             # Real environment - check actual file existence
             exists = os.path.isfile(path)
             executable = os.access(path, os.X_OK)
             checked_details.append(f"{path} (exists: {exists}, executable: {executable})")
-            logging.debug(f"Checking SDK manager path for {for_purpose}: {path} - exists: {exists}, executable: {executable}")
+            log.debug(f"Checking SDK manager path for {for_purpose}: {path} - exists: {exists}, executable: {executable}")
 
             if exists and executable:
                 sdk_manager = path
-                logging.info(f"Found SDK manager for {for_purpose} at: {sdk_manager}")
+                log.info(f"Found SDK manager for {for_purpose} at: {sdk_manager}")
                 break
 
         return sdk_manager, checked_details
@@ -203,16 +205,16 @@ def _handle_sdk_license_acceptance(self):
                 title="Accepting NDK licenses using: %s" % sdk_manager_for_licenses,
             )
         else:
-            logging.warning("No SDK manager found for license acceptance - licenses may need to be accepted manually")
+            log.warning("No SDK manager found for license acceptance - licenses may need to be accepted manually")
 
     def validate_build_environment(self):
         # Log Android environment paths for debugging
         android_ndk_home = os.environ.get("ANDROID_NDK_HOME", "")
         android_home = os.environ.get("ANDROID_HOME", "")
 
-        logging.info("Android environment paths:")
-        logging.info(f"  ANDROID_NDK_HOME: {android_ndk_home}")
-        logging.info(f"  ANDROID_HOME: {android_home}")
+        log.info("Android environment paths:")
+        log.info(f"  ANDROID_NDK_HOME: {android_ndk_home}")
+        log.info(f"  ANDROID_HOME: {android_home}")
 
         for k in ["ANDROID_NDK_HOME", "ANDROID_HOME"]:
             if k not in os.environ:
@@ -225,9 +227,9 @@ def validate_build_environment(self):
         sdk_manager, checked_details = self._find_sdk_manager("validation")
 
         if not sdk_manager:
-            logging.error("SDK manager not found in any expected location")
+            log.error("SDK manager not found in any expected location")
             for detail in checked_details:
-                logging.error(f"  {detail}")
+                log.error(f"  {detail}")
 
             android_home = os.environ["ANDROID_HOME"]
             possible_fixes = [
@@ -314,7 +316,7 @@ def copyToSrcAndroid(self):
             "CHIPClusterID.jar": "src/controller/java/CHIPClusterID.jar",
         }
 
-        for jarName in jars.keys():
+        for jarName in jars:
             self._Execute(
                 [
                     "cp",
@@ -339,7 +341,7 @@ def copyToExampleApp(self, jnilibs_dir, libs_dir, libs, jars):
                 ]
             )
 
-        for jarName in jars.keys():
+        for jarName in jars:
             self._Execute(
                 [
                     "cp",