Improved file filtering for the Static Analysis feature (#4570)

* Improved file filtering for the Static Analysis feature

* Better handling of legacy "check_patterns" option

* Rename "check_src_filter" to plural form to better represent functionality

* Move to plural forms of filter variables

HISTORY.rst

@@ -20,6 +20,7 @@ PlatformIO Core 6
 * Show detailed library dependency tree only in the `verbose mode <https://docs.platformio.org/en/latest/core/userguide/cmd_run.html#cmdoption-pio-run-v>`__ (`issue #4517 <https://github.com/platformio/platformio-core/issues/4517>`_)
 * Prevented shell injection when converting INO file to CPP (`issue #4532 <https://github.com/platformio/platformio-core/issues/4532>`_)
 * Restored project generator for `NetBeans IDE <https://docs.platformio.org/en/latest/integration/ide/netbeans.html>`__
+* Improved source file filtering functionality for `Static Code Analysis <https://docs.platformio.org/en/latest/advanced/static-code-analysis/index.html>`__ feature
 
 6.1.6 (2023-01-23)
 ~~~~~~~~~~~~~~~~~~

platformio/check/cli.py

@@ -49,7 +49,12 @@ from platformio.project.helpers import find_project_dir_above, get_project_dir
         exists=True, file_okay=True, dir_okay=False, readable=True, resolve_path=True
     ),
 )
-@click.option("--pattern", multiple=True)
+@click.option("--pattern", multiple=True, hidden=True)
+@click.option(
+    "-f",
+    "--src-filters",
+    multiple=True
+)
 @click.option("--flags", multiple=True)
 @click.option(
     "--severity", multiple=True, type=click.Choice(DefectItem.SEVERITY_LABELS.values())
@@ -67,6 +72,7 @@ def cli(
     environment,
     project_dir,
     project_conf,
+    src_filters,
     pattern,
     flags,
     severity,
@@ -105,14 +111,24 @@ def cli(
                     "%s: %s" % (k, ", ".join(v) if isinstance(v, list) else v)
                 )
 
-            default_patterns = [
-                config.get("platformio", "src_dir"),
-                config.get("platformio", "include_dir"),
+            default_src_filters = [
+                "+<%s>" % os.path.basename(config.get("platformio", "src_dir")),
+                "+<%s>" % os.path.basename(config.get("platformio", "include_dir")),
             ]
+
+            src_filters = (
+                src_filters
+                or pattern
+                or env_options.get(
+                    "check_src_filters",
+                    env_options.get("check_patterns", default_src_filters),
+                )
+            )
+
             tool_options = dict(
                 verbose=verbose,
                 silent=silent,
-                patterns=pattern or env_options.get("check_patterns", default_patterns),
+                src_filters=src_filters,
                 flags=flags or env_options.get("check_flags"),
                 severity=[DefectItem.SEVERITY_LABELS[DefectItem.SEVERITY_HIGH]]
                 if silent
@@ -265,7 +281,7 @@ def print_defects_stats(results):
     tabular_data.append(total)
 
     headers = ["Component"]
-    headers.extend([l.upper() for l in severity_labels])
+    headers.extend([label.upper() for label in severity_labels])
     headers = [click.style(h, bold=True) for h in headers]
     click.echo(tabulate(tabular_data, headers=headers, numalign="center"))
     click.echo()

platformio/check/tools/base.py

@@ -12,7 +12,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import glob
 import os
 import tempfile
 
@@ -30,6 +29,7 @@ class CheckToolBase:  # pylint: disable=too-many-instance-attributes
         self.config = config
         self.envname = envname
         self.options = options
+        self.project_dir = project_dir
         self.cc_flags = []
         self.cxx_flags = []
         self.cpp_includes = []
@@ -41,7 +41,7 @@ class CheckToolBase:  # pylint: disable=too-many-instance-attributes
         self._defects = []
         self._on_defect_callback = None
         self._bad_input = False
-        self._load_cpp_data(project_dir)
+        self._load_cpp_data()
 
         # detect all defects by default
         if not self.options.get("severity"):
@@ -56,8 +56,8 @@ class CheckToolBase:  # pylint: disable=too-many-instance-attributes
             for s in self.options["severity"]
         ]
 
-    def _load_cpp_data(self, project_dir):
-        data = load_build_metadata(project_dir, self.envname)
+    def _load_cpp_data(self):
+        data = load_build_metadata(self.project_dir, self.envname)
         if not data:
             return
         self.cc_flags = click.parser.split_arg_string(data.get("cc_flags", ""))
@@ -99,6 +99,13 @@ class CheckToolBase:  # pylint: disable=too-many-instance-attributes
                 includes_file,
             )
             result = proc.exec_command(cmd, shell=True)
+
+            if result["returncode"] != 0:
+                click.echo("Warning: Failed to extract toolchain defines!")
+                if self.options.get("verbose"):
+                    click.echo(result["out"])
+                    click.echo(result["err"])
+
             for line in result["out"].split("\n"):
                 tokens = line.strip().split(" ", 2)
                 if not tokens or tokens[0] != "#define":
@@ -201,7 +208,7 @@ class CheckToolBase:  # pylint: disable=too-many-instance-attributes
         return result
 
     @staticmethod
-    def get_project_target_files(patterns):
+    def get_project_target_files(project_dir, src_filters):
         c_extension = (".c",)
         cpp_extensions = (".cc", ".cpp", ".cxx", ".ino")
         header_extensions = (".h", ".hh", ".hpp", ".hxx")
@@ -216,13 +223,9 @@ class CheckToolBase:  # pylint: disable=too-many-instance-attributes
             elif path.endswith(cpp_extensions):
                 result["c++"].append(os.path.abspath(path))
 
-        for pattern in patterns:
-            for item in glob.glob(pattern, recursive=True):
-                if not os.path.isdir(item):
-                    _add_file(item)
-                for root, _, files in os.walk(item, followlinks=True):
-                    for f in files:
-                        _add_file(os.path.join(root, f))
+        src_filters = normalize_src_filters(src_filters)
+        for f in fs.match_src_files(project_dir, src_filters):
+            _add_file(f)
 
         return result
 
@@ -243,3 +246,20 @@ class CheckToolBase:  # pylint: disable=too-many-instance-attributes
         self.clean_up()
 
         return self._bad_input
+
+
+#
+# Helpers
+#
+
+
+def normalize_src_filters(src_filters):
+    def _normalize(src_filters):
+        return (
+            src_filters if src_filters.startswith(("+<", "-<")) else "+<%s>" % src_filters
+        )
+
+    if isinstance(src_filters, (list, tuple)):
+        return " ".join([_normalize(f) for f in src_filters])
+
+    return _normalize(src_filters)

platformio/check/tools/clangtidy.py

@@ -64,7 +64,9 @@ class ClangtidyCheckTool(CheckToolBase):
         ):
             cmd.append("--checks=*")
 
-        project_files = self.get_project_target_files(self.options["patterns"])
+        project_files = self.get_project_target_files(
+            self.project_dir, self.options["src_filters"]
+        )
 
         src_files = []
         for items in project_files.values():

platformio/check/tools/cppcheck.py

@@ -96,7 +96,7 @@ class CppcheckCheckTool(CheckToolBase):
                     )
                     click.echo()
                 self._bad_input = True
-                self._buffer = ""
+            self._buffer = ""
             return None
 
         self._buffer = ""
@@ -214,7 +214,9 @@ class CppcheckCheckTool(CheckToolBase):
         if not self.is_flag_set("--addon", self.get_flags("cppcheck")):
             return
 
-        for files in self.get_project_target_files(self.options["patterns"]).values():
+        for files in self.get_project_target_files(
+            self.project_dir, self.options["src_filters"]
+        ).values():
             for f in files:
                 dump_file = f + ".dump"
                 if os.path.isfile(dump_file):
@@ -243,7 +245,9 @@ class CppcheckCheckTool(CheckToolBase):
     def check(self, on_defect_callback=None):
         self._on_defect_callback = on_defect_callback
 
-        project_files = self.get_project_target_files(self.options["patterns"])
+        project_files = self.get_project_target_files(
+            self.project_dir, self.options["src_filters"]
+        )
         src_files_scope = ("c", "c++")
         if not any(project_files[t] for t in src_files_scope):
             click.echo("Error: Nothing to check.")

platformio/check/tools/pvsstudio.py

@@ -227,7 +227,7 @@ class PvsStudioCheckTool(CheckToolBase):  # pylint: disable=too-many-instance-at
     def check(self, on_defect_callback=None):
         self._on_defect_callback = on_defect_callback
         for scope, files in self.get_project_target_files(
-            self.options["patterns"]
+            self.project_dir, self.options["src_filters"]
         ).items():
             if scope not in ("c", "c++"):
                 continue

platformio/project/options.py

@@ -649,7 +649,8 @@ ProjectOptions = OrderedDict(
             ),
             ConfigEnvOption(
                 group="check",
-                name="check_patterns",
+                name="check_src_filters",
+                oldnames=["check_patterns"],
                 description=(
                     "Configure a list of target files or directories for checking "
                     "(Unix shell-style wildcards)"

tests/commands/test_check.py

@@ -15,8 +15,8 @@
 # pylint: disable=redefined-outer-name
 
 import json
+import os
 import sys
-from os.path import isfile, join
 
 import pytest
 
@@ -83,12 +83,12 @@ def check_dir(tmpdir_factory):
 
 def count_defects(output):
     error, warning, style = 0, 0, 0
-    for l in output.split("\n"):
-        if "[high:error]" in l:
+    for line in output.split("\n"):
+        if "[high:error]" in line:
             error += 1
-        elif "[medium:warning]" in l:
+        elif "[medium:warning]" in line:
             warning += 1
-        elif "[low:style]" in l:
+        elif "[low:style]" in line:
             style += 1
     return error, warning, style
 
@@ -240,9 +240,9 @@ def test_check_includes_passed(clirunner, check_dir):
     result = clirunner.invoke(cmd_check, ["--project-dir", str(check_dir), "--verbose"])
 
     inc_count = 0
-    for l in result.output.split("\n"):
-        if l.startswith("Includes:"):
-            inc_count = l.count("-I")
+    for line in result.output.split("\n"):
+        if line.startswith("Includes:"):
+            inc_count = line.count("-I")
 
     # at least 1 include path for default mode
     assert inc_count > 0
@@ -259,46 +259,6 @@ def test_check_silent_mode(clirunner, validate_cliresult, check_dir):
     assert style == 0
 
 
-def test_check_custom_pattern_absolute_path(
-    clirunner, validate_cliresult, tmpdir_factory
-):
-    project_dir = tmpdir_factory.mktemp("project")
-    project_dir.join("platformio.ini").write(DEFAULT_CONFIG)
-
-    check_dir = tmpdir_factory.mktemp("custom_src_dir")
-    check_dir.join("main.cpp").write(TEST_CODE)
-
-    result = clirunner.invoke(
-        cmd_check, ["--project-dir", str(project_dir), "--pattern=" + str(check_dir)]
-    )
-    validate_cliresult(result)
-
-    errors, warnings, style = count_defects(result.output)
-
-    assert errors == EXPECTED_ERRORS
-    assert warnings == EXPECTED_WARNINGS
-    assert style == EXPECTED_STYLE
-
-
-def test_check_custom_pattern_relative_path(
-    clirunner, validate_cliresult, tmpdir_factory
-):
-    tmpdir = tmpdir_factory.mktemp("project")
-    tmpdir.join("platformio.ini").write(DEFAULT_CONFIG)
-
-    tmpdir.mkdir("app").join("main.cpp").write(TEST_CODE)
-    tmpdir.mkdir("prj").join("test.cpp").write(TEST_CODE)
-
-    result = clirunner.invoke(
-        cmd_check, ["--project-dir", str(tmpdir), "--pattern=app", "--pattern=prj"]
-    )
-    validate_cliresult(result)
-
-    errors, warnings, style = count_defects(result.output)
-
-    assert errors + warnings + style == EXPECTED_DEFECTS * 2
-
-
 def test_check_no_source_files(clirunner, tmpdir):
     tmpdir.join("platformio.ini").write(DEFAULT_CONFIG)
     tmpdir.mkdir("src")
@@ -427,7 +387,7 @@ R21.4 text.
 
     validate_cliresult(result)
     assert "R21.3 Found MISRA defect" in result.output
-    assert not isfile(join(str(check_dir), "src", "main.cpp.dump"))
+    assert not os.path.isfile(os.path.join(str(check_dir), "src", "main.cpp.dump"))
 
 
 def test_check_fails_on_defects_only_with_flag(clirunner, validate_cliresult, tmpdir):
@@ -607,10 +567,10 @@ framework = arduino
     validate_cliresult(result)
 
     project_path = fs.to_unix_path(str(tmpdir))
-    for l in result.output.split("\n"):
-        if not l.startswith("Includes:"):
+    for line in result.output.split("\n"):
+        if not line.startswith("Includes:"):
             continue
-        for inc in l.split(" "):
+        for inc in line.split(" "):
             if inc.startswith("-I") and project_path not in inc:
                 pytest.fail("Detected an include path from packages: " + inc)
 
@@ -656,3 +616,147 @@ def test_check_handles_spaces_in_paths(clirunner, validate_cliresult, tmpdir_fac
     default_result = clirunner.invoke(cmd_check, ["--project-dir", str(tmpdir)])
 
     validate_cliresult(default_result)
+
+
+#
+# Files filtering functionality
+#
+
+
+@pytest.mark.parametrize(
+    "src_filter,number_of_checked_files",
+    [
+        (["+<src/app.cpp>"], 1),
+        (["+<tests/*.cpp>"], 1),
+        (["+<src>", "-<src/*.cpp>"], 2),
+        (["-<*> +<src/main.cpp> +<src/uart/uart.cpp> +<src/spi/spi.cpp>"], 3),
+    ],
+    ids=["Single file", "Glob pattern", "Exclude pattern", "Filter as string"],
+)
+def test_check_src_filter(
+    clirunner,
+    validate_cliresult,
+    tmpdir_factory,
+    src_filter,
+    number_of_checked_files,
+):
+    tmpdir = tmpdir_factory.mktemp("project")
+    tmpdir.join("platformio.ini").write(DEFAULT_CONFIG)
+
+    src_dir = tmpdir.mkdir("src")
+    src_dir.join("main.cpp").write(TEST_CODE)
+    src_dir.join("app.cpp").write(TEST_CODE)
+    src_dir.mkdir("uart").join("uart.cpp").write(TEST_CODE)
+    src_dir.mkdir("spi").join("spi.cpp").write(TEST_CODE)
+    tmpdir.mkdir("tests").join("test.cpp").write(TEST_CODE)
+
+    cmd_args = ["--project-dir", str(tmpdir)] + [
+        "--src-filters=%s" % f for f in src_filter
+    ]
+
+    result = clirunner.invoke(cmd_check, cmd_args)
+    validate_cliresult(result)
+
+    errors, warnings, style = count_defects(result.output)
+
+    assert errors + warnings + style == EXPECTED_DEFECTS * number_of_checked_files
+
+
+def test_check_src_filter_from_config(clirunner, validate_cliresult, tmpdir_factory):
+    tmpdir = tmpdir_factory.mktemp("project")
+
+    config = (
+        DEFAULT_CONFIG
+        + """
+check_src_filters =
+    +<src/spi/*.c*>
+    +<tests/test.cpp>
+    """
+    )
+    tmpdir.join("platformio.ini").write(config)
+
+    src_dir = tmpdir.mkdir("src")
+    src_dir.join("main.cpp").write(TEST_CODE)
+    src_dir.mkdir("spi").join("spi.cpp").write(TEST_CODE)
+    tmpdir.mkdir("tests").join("test.cpp").write(TEST_CODE)
+
+    result = clirunner.invoke(cmd_check, ["--project-dir", str(tmpdir)])
+    validate_cliresult(result)
+
+    errors, warnings, style = count_defects(result.output)
+
+    assert errors + warnings + style == EXPECTED_DEFECTS * 2
+    assert "main.cpp" not in result.output
+
+
+def test_check_custom_pattern_absolute_path_legacy(
+    clirunner, validate_cliresult, tmpdir_factory
+):
+    project_dir = tmpdir_factory.mktemp("project")
+    project_dir.join("platformio.ini").write(DEFAULT_CONFIG)
+
+    check_dir = tmpdir_factory.mktemp("custom_src_dir")
+    check_dir.join("main.cpp").write(TEST_CODE)
+
+    result = clirunner.invoke(
+        cmd_check, ["--project-dir", str(project_dir), "--pattern=" + str(check_dir)]
+    )
+
+    validate_cliresult(result)
+
+    errors, warnings, style = count_defects(result.output)
+
+    assert errors == EXPECTED_ERRORS
+    assert warnings == EXPECTED_WARNINGS
+    assert style == EXPECTED_STYLE
+
+
+def test_check_custom_pattern_relative_path_legacy(
+    clirunner, validate_cliresult, tmpdir_factory
+):
+    tmpdir = tmpdir_factory.mktemp("project")
+    tmpdir.join("platformio.ini").write(DEFAULT_CONFIG)
+
+    src_dir = tmpdir.mkdir("src")
+    src_dir.join("main.cpp").write(TEST_CODE)
+    src_dir.mkdir("uart").join("uart.cpp").write(TEST_CODE)
+    src_dir.mkdir("spi").join("spi.cpp").write(TEST_CODE)
+
+    result = clirunner.invoke(
+        cmd_check,
+        ["--project-dir", str(tmpdir), "--pattern=src/uart", "--pattern=src/spi"],
+    )
+    validate_cliresult(result)
+
+    errors, warnings, style = count_defects(result.output)
+
+    assert errors + warnings + style == EXPECTED_DEFECTS * 2
+
+
+def test_check_src_filter_from_config_legacy(
+    clirunner, validate_cliresult, tmpdir_factory
+):
+    tmpdir = tmpdir_factory.mktemp("project")
+
+    config = (
+        DEFAULT_CONFIG
+        + """
+check_patterns =
+    src/spi/*.c*
+    tests/test.cpp
+    """
+    )
+    tmpdir.join("platformio.ini").write(config)
+
+    src_dir = tmpdir.mkdir("src")
+    src_dir.join("main.cpp").write(TEST_CODE)
+    src_dir.mkdir("spi").join("spi.cpp").write(TEST_CODE)
+    tmpdir.mkdir("tests").join("test.cpp").write(TEST_CODE)
+
+    result = clirunner.invoke(cmd_check, ["--project-dir", str(tmpdir)])
+    validate_cliresult(result)
+
+    errors, warnings, style = count_defects(result.output)
+
+    assert errors + warnings + style == EXPECTED_DEFECTS * 2
+    assert "main.cpp" not in result.output