Updates for PIO Check (#3640)

* Update check tools to the latest versions

* Use language standard when exporting defines to check tools

* Buffer Cppcheck output to detect multiline messages

* Add new test for PIO Check

* Pass include paths to Clang-Tidy as individual compiler arguments

Clang-tidy doesn't support response files which can exceed command
length limitations on Windows

* Simplify tests for PIO Check

* Update history

* Sync changelog

HISTORY.rst

@@ -82,6 +82,7 @@ PlatformIO Core 4
 * Added `PlatformIO CLI Shell Completion <https://docs.platformio.org/page/core/userguide/system/completion/index.html>`__ for Fish, Zsh, Bash, and PowerShell (`issue #3435 <https://github.com/platformio/platformio-core/issues/3435>`_)
 * Automatically build ``contrib-pysite`` package on a target machine when pre-built package is not compatible (`issue #3482 <https://github.com/platformio/platformio-core/issues/3482>`_)
 * Fixed an issue on Windows when installing a library dependency from Git repository (`issue #2844 <https://github.com/platformio/platformio-core/issues/2844>`_, `issue #3328 <https://github.com/platformio/platformio-core/issues/3328>`_)
+* Fixed an issue with PIO Check when a defect with multiline error message is not reported in verbose mode (`issue #3631 <https://github.com/platformio/platformio-core/issues/3631>`_)
 
 4.3.3 (2020-04-28)
 ~~~~~~~~~~~~~~~~~~

platformio/__init__.py

@@ -51,9 +51,9 @@ __core_packages__ = {
     "contrib-pysite": "~2.%d%d.0" % (sys.version_info.major, sys.version_info.minor),
     "tool-unity": "~1.20500.0",
     "tool-scons": "~2.20501.7" if sys.version_info.major == 2 else "~4.40001.0",
-    "tool-cppcheck": "~1.190.0",
+    "tool-cppcheck": "~1.210.0",
     "tool-clangtidy": "~1.100000.0",
-    "tool-pvs-studio": "~7.7.0",
+    "tool-pvs-studio": "~7.8.0",
 }
 
 __check_internet_hosts__ = [

platformio/commands/check/tools/base.py

@@ -83,7 +83,9 @@ class CheckToolBase(object):  # pylint: disable=too-many-instance-attributes
             cmd = "echo | %s -x %s %s %s -dM -E -" % (
                 self.cc_path,
                 language,
-                " ".join([f for f in build_flags if f.startswith(("-m", "-f"))]),
+                " ".join(
+                    [f for f in build_flags if f.startswith(("-m", "-f", "-std"))]
+                ),
                 includes_file,
             )
             result = proc.exec_command(cmd, shell=True)

platformio/commands/check/tools/clangtidy.py

@@ -63,10 +63,7 @@ class ClangtidyCheckTool(CheckToolBase):
         for scope in project_files:
             src_files.extend(project_files[scope])
 
-        cmd.extend(flags)
+        cmd.extend(flags + src_files + ["--"])
-        cmd.extend(src_files)
-        cmd.append("--")
-
         cmd.extend(
             ["-D%s" % d for d in self.cpp_defines + self.toolchain_defines["c++"]]
         )
@@ -79,6 +76,6 @@ class ClangtidyCheckTool(CheckToolBase):
                 continue
             includes.append(inc)
 
-        cmd.append("--extra-arg=" + self._long_includes_hook(includes))
+        cmd.extend(["-I%s" % inc for inc in includes])
 
         return cmd

platformio/commands/check/tools/cppcheck.py

@@ -24,6 +24,8 @@ from platformio.package.manager.core import get_core_package_dir
 
 class CppcheckCheckTool(CheckToolBase):
     def __init__(self, *args, **kwargs):
+        self._field_delimiter = "<&PIO&>"
+        self._buffer = ""
         self.defect_fields = [
             "severity",
             "message",
@@ -55,13 +57,15 @@ class CppcheckCheckTool(CheckToolBase):
         return line
 
     def parse_defect(self, raw_line):
-        if "<&PIO&>" not in raw_line or any(
+        if self._field_delimiter not in raw_line:
-            f not in raw_line for f in self.defect_fields
+            return None
-        ):
+
+        self._buffer += raw_line
+        if any(f not in self._buffer for f in self.defect_fields):
             return None
 
         args = dict()
-        for field in raw_line.split("<&PIO&>"):
+        for field in self._buffer.split(self._field_delimiter):
             field = field.strip().replace('"', "")
             name, value = field.split("=", 1)
             args[name] = value
@@ -94,6 +98,7 @@ class CppcheckCheckTool(CheckToolBase):
                 self._bad_input = True
             return None
 
+        self._buffer = ""
         return DefectItem(**args)
 
     def configure_command(
@@ -103,13 +108,16 @@ class CppcheckCheckTool(CheckToolBase):
 
         cmd = [
             tool_path,
+            "--addon-python=%s" % proc.get_pythonexe_path(),
             "--error-exitcode=1",
             "--verbose" if self.options.get("verbose") else "--quiet",
         ]
 
         cmd.append(
             '--template="%s"'
-            % "<&PIO&>".join(["{0}={{{0}}}".format(f) for f in self.defect_fields])
+            % self._field_delimiter.join(
+                ["{0}={{{0}}}".format(f) for f in self.defect_fields]
+            )
         )
 
         flags = self.get_flags("cppcheck")

tests/commands/test_check.py

@@ -61,6 +61,12 @@ int main() {
 }
 """
 
+
+PVS_STUDIO_FREE_LICENSE_HEADER = """
+// This is an open source non-commercial project. Dear PVS-Studio, please check it.
+// PVS-Studio Static Code Analyzer for C, C++, C#, and Java: http://www.viva64.com
+"""
+
 EXPECTED_ERRORS = 4
 EXPECTED_WARNINGS = 1
 EXPECTED_STYLE = 1
@@ -87,19 +93,21 @@ def count_defects(output):
     return error, warning, style
 
 
-def test_check_cli_output(clirunner, check_dir):
+def test_check_cli_output(clirunner, validate_cliresult, check_dir):
     result = clirunner.invoke(cmd_check, ["--project-dir", str(check_dir)])
+    validate_cliresult(result)
 
     errors, warnings, style = count_defects(result.output)
 
-    assert result.exit_code == 0
     assert errors + warnings + style == EXPECTED_DEFECTS
 
 
-def test_check_json_output(clirunner, check_dir):
+def test_check_json_output(clirunner, validate_cliresult, check_dir):
     result = clirunner.invoke(
         cmd_check, ["--project-dir", str(check_dir), "--json-output"]
     )
+    validate_cliresult(result)
+
     output = json.loads(result.stdout.strip())
 
     assert isinstance(output, list)
@@ -114,14 +122,24 @@ def test_check_tool_defines_passed(clirunner, check_dir):
     assert "__GNUC__" in output
 
 
-def test_check_severity_threshold(clirunner, check_dir):
+def test_check_language_standard_definition_passed(clirunner, tmpdir):
+    config = DEFAULT_CONFIG + "\nbuild_flags = -std=c++17"
+    tmpdir.join("platformio.ini").write(config)
+    tmpdir.mkdir("src").join("main.cpp").write(TEST_CODE)
+    result = clirunner.invoke(cmd_check, ["--project-dir", str(tmpdir), "-v"])
+
+    assert "__cplusplus=201703L" in result.output
+    assert "--std=c++17" in result.output
+
+
+def test_check_severity_threshold(clirunner, validate_cliresult, check_dir):
     result = clirunner.invoke(
         cmd_check, ["--project-dir", str(check_dir), "--severity=high"]
     )
+    validate_cliresult(result)
 
     errors, warnings, style = count_defects(result.output)
 
-    assert result.exit_code == 0
     assert errors == EXPECTED_ERRORS
     assert warnings == 0
     assert style == 0
@@ -129,10 +147,9 @@ def test_check_severity_threshold(clirunner, check_dir):
 
 def test_check_includes_passed(clirunner, check_dir):
     result = clirunner.invoke(cmd_check, ["--project-dir", str(check_dir), "--verbose"])
-    output = result.output
 
     inc_count = 0
-    for l in output.split("\n"):
+    for l in result.output.split("\n"):
         if l.startswith("Includes:"):
             inc_count = l.count("-I")
 
@@ -140,18 +157,18 @@ def test_check_includes_passed(clirunner, check_dir):
     assert inc_count > 1
 
 
-def test_check_silent_mode(clirunner, check_dir):
+def test_check_silent_mode(clirunner, validate_cliresult, check_dir):
     result = clirunner.invoke(cmd_check, ["--project-dir", str(check_dir), "--silent"])
+    validate_cliresult(result)
 
     errors, warnings, style = count_defects(result.output)
 
-    assert result.exit_code == 0
     assert errors == EXPECTED_ERRORS
     assert warnings == 0
     assert style == 0
 
 
-def test_check_custom_pattern_absolute_path(clirunner, tmpdir_factory):
+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)
 
@@ -161,16 +178,16 @@ def test_check_custom_pattern_absolute_path(clirunner, tmpdir_factory):
     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 result.exit_code == 0
     assert errors == EXPECTED_ERRORS
     assert warnings == EXPECTED_WARNINGS
     assert style == EXPECTED_STYLE
 
 
-def test_check_custom_pattern_relative_path(clirunner, tmpdir_factory):
+def test_check_custom_pattern_relative_path(clirunner, validate_cliresult, tmpdir_factory):
     tmpdir = tmpdir_factory.mktemp("project")
     tmpdir.join("platformio.ini").write(DEFAULT_CONFIG)
 
@@ -180,10 +197,10 @@ def test_check_custom_pattern_relative_path(clirunner, tmpdir_factory):
     result = clirunner.invoke(
         cmd_check, ["--project-dir", str(tmpdir), "--pattern=app", "--pattern=prj"]
     )
+    validate_cliresult(result)
 
     errors, warnings, style = count_defects(result.output)
 
-    assert result.exit_code == 0
     assert errors + warnings + style == EXPECTED_DEFECTS * 2
 
 
@@ -214,7 +231,7 @@ def test_check_bad_flag_passed(clirunner, check_dir):
     assert style == 0
 
 
-def test_check_success_if_no_errors(clirunner, tmpdir):
+def test_check_success_if_no_errors(clirunner, validate_cliresult, tmpdir):
     tmpdir.join("platformio.ini").write(DEFAULT_CONFIG)
     tmpdir.mkdir("src").join("main.c").write(
         """
@@ -232,26 +249,28 @@ int main() {
     )
 
     result = clirunner.invoke(cmd_check, ["--project-dir", str(tmpdir)])
+    validate_cliresult(result)
 
     errors, warnings, style = count_defects(result.output)
 
     assert "[PASSED]" in result.output
-    assert result.exit_code == 0
     assert errors == 0
     assert warnings == 1
     assert style == 1
 
 
-def test_check_individual_flags_passed(clirunner, tmpdir):
+def test_check_individual_flags_passed(clirunner, validate_cliresult, tmpdir):
     config = DEFAULT_CONFIG + "\ncheck_tool = cppcheck, clangtidy, pvs-studio"
     config += """\ncheck_flags =
     cppcheck: --std=c++11
     clangtidy: --fix-errors
     pvs-studio: --analysis-mode=4
 """
+
     tmpdir.join("platformio.ini").write(config)
-    tmpdir.mkdir("src").join("main.cpp").write(TEST_CODE)
+    tmpdir.mkdir("src").join("main.cpp").write(PVS_STUDIO_FREE_LICENSE_HEADER + TEST_CODE)
     result = clirunner.invoke(cmd_check, ["--project-dir", str(tmpdir), "-v"])
+    validate_cliresult(result)
 
     clang_flags_found = cppcheck_flags_found = pvs_flags_found = False
     for l in result.output.split("\n"):
@@ -269,7 +288,7 @@ def test_check_individual_flags_passed(clirunner, tmpdir):
     assert pvs_flags_found
 
 
-def test_check_cppcheck_misra_addon(clirunner, check_dir):
+def test_check_cppcheck_misra_addon(clirunner, validate_cliresult, check_dir):
     check_dir.join("misra.json").write(
         """
 {
@@ -309,12 +328,12 @@ R21.4 text.
         cmd_check, ["--project-dir", str(check_dir), "--flags=--addon=misra.json"]
     )
 
-    assert result.exit_code == 0
+    validate_cliresult(result)
     assert "R21.3 Found MISRA defect" in result.output
     assert not isfile(join(str(check_dir), "src", "main.cpp.dump"))
 
 
-def test_check_fails_on_defects_only_with_flag(clirunner, tmpdir):
+def test_check_fails_on_defects_only_with_flag(clirunner, validate_cliresult, tmpdir):
     config = DEFAULT_CONFIG + "\ncheck_tool = cppcheck, clangtidy"
     tmpdir.join("platformio.ini").write(config)
     tmpdir.mkdir("src").join("main.cpp").write(TEST_CODE)
@@ -325,11 +344,13 @@ def test_check_fails_on_defects_only_with_flag(clirunner, tmpdir):
         cmd_check, ["--project-dir", str(tmpdir), "--fail-on-defect=high"]
     )
 
-    assert default_result.exit_code == 0
+    validate_cliresult(default_result)
     assert result_with_flag.exit_code != 0
 
 
-def test_check_fails_on_defects_only_on_specified_level(clirunner, tmpdir):
+def test_check_fails_on_defects_only_on_specified_level(
+    clirunner, validate_cliresult, tmpdir
+):
     config = DEFAULT_CONFIG + "\ncheck_tool = cppcheck, clangtidy"
     tmpdir.join("platformio.ini").write(config)
     tmpdir.mkdir("src").join("main.c").write(
@@ -350,12 +371,12 @@ int main() {
     high_result = clirunner.invoke(
         cmd_check, ["--project-dir", str(tmpdir), "--fail-on-defect=high"]
     )
+    validate_cliresult(high_result)
 
     low_result = clirunner.invoke(
         cmd_check, ["--project-dir", str(tmpdir), "--fail-on-defect=low"]
     )
 
-    assert high_result.exit_code == 0
     assert low_result.exit_code != 0
 
 
@@ -367,15 +388,9 @@ board = teensy35
 framework = arduino
 check_tool = pvs-studio
 """
-    code = (
-        """// This is an open source non-commercial project. Dear PVS-Studio, please check it.
-// PVS-Studio Static Code Analyzer for C, C++, C#, and Java: http://www.viva64.com
-"""
-        + TEST_CODE
-    )
 
     tmpdir.join("platformio.ini").write(config)
-    tmpdir.mkdir("src").join("main.c").write(code)
+    tmpdir.mkdir("src").join("main.c").write(PVS_STUDIO_FREE_LICENSE_HEADER + TEST_CODE)
 
     result = clirunner.invoke(
         cmd_check, ["--project-dir", str(tmpdir), "--fail-on-defect=high", "-v"]
@@ -399,8 +414,7 @@ check_tool = %s
 """
     # tmpdir.join("platformio.ini").write(config)
     tmpdir.mkdir("src").join("main.c").write(
-        """// This is an open source non-commercial project. Dear PVS-Studio, please check it.
+        PVS_STUDIO_FREE_LICENSE_HEADER + """
-// PVS-Studio Static Code Analyzer for C, C++, C#, and Java: http://www.viva64.com
 #include <stdlib.h>
 
 void unused_function(int val){
@@ -425,13 +439,13 @@ int main() {
             result = clirunner.invoke(cmd_check, ["--project-dir", str(tmpdir)])
             validate_cliresult(result)
             defects = sum(count_defects(result.output))
-            assert result.exit_code == 0 and defects > 0, "Failed %s with %s" % (
+            assert defects > 0, "Failed %s with %s" % (
                 framework,
                 tool,
             )
 
 
-def test_check_skip_includes_from_packages(clirunner, tmpdir):
+def test_check_skip_includes_from_packages(clirunner, validate_cliresult, tmpdir):
     config = """
 [env:test]
 platform = nordicnrf52
@@ -445,13 +459,42 @@ framework = arduino
     result = clirunner.invoke(
         cmd_check, ["--project-dir", str(tmpdir), "--skip-packages", "-v"]
     )
-
+    validate_cliresult(result)
-    output = result.output
 
     project_path = fs.to_unix_path(str(tmpdir))
-    for l in output.split("\n"):
+    for l in result.output.split("\n"):
         if not l.startswith("Includes:"):
             continue
         for inc in l.split(" "):
             if inc.startswith("-I") and project_path not in inc:
                 pytest.fail("Detected an include path from packages: " + inc)
+
+
+def test_check_multiline_error(clirunner, tmpdir_factory):
+    project_dir = tmpdir_factory.mktemp("project")
+    project_dir.join("platformio.ini").write(DEFAULT_CONFIG)
+
+    project_dir.mkdir("include").join("main.h").write(
+        """
+#error This is a multiline error message \\
+that should be correctly reported \\
+in both default and verbose modes.
+"""
+    )
+
+    project_dir.mkdir("src").join("main.c").write(
+        """
+#include <stdlib.h>
+#include "main.h"
+
+int main() {}
+"""
+    )
+
+    result = clirunner.invoke(cmd_check, ["--project-dir", str(project_dir)])
+    errors, _, _ = count_defects(result.output)
+
+    result = clirunner.invoke(cmd_check, ["--project-dir", str(project_dir), "-v"])
+    verbose_errors, _, _ = count_defects(result.output)
+
+    assert verbose_errors == errors == 1