got rid of duplicated error reporting code in CppCheck::checkInternal() / cleanups (#7960)

Author: firewave

lib/cppcheck.cpp

@@ -977,30 +977,11 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
         std::vector<std::string> files;
         simplecpp::TokenList tokens1 = createTokenList(files, &outputList);
 
-        // If there is a syntax error, report it and stop
-        const auto output_it = std::find_if(outputList.cbegin(), outputList.cend(), [](const simplecpp::Output &output){
-            return Preprocessor::hasErrors(output);
-        });
-        if (output_it != outputList.cend()) {
-            const simplecpp::Output &output = *output_it;
-            std::string locfile = Path::fromNativeSeparators(output.location.file());
-            if (mSettings.relativePaths)
-                locfile = Path::getRelativePath(locfile, mSettings.basePaths);
-
-            ErrorMessage::FileLocation loc1(locfile, output.location.line, output.location.col);
-
-            ErrorMessage errmsg({std::move(loc1)},
-                                "", // TODO: is this correct?
-                                Severity::error,
-                                output.msg,
-                                "syntaxError",
-                                Certainty::normal);
-            mErrorLogger.reportErr(errmsg);
-            return mLogger->exitcode();
-        }
-
         Preprocessor preprocessor(tokens1, mSettings, mErrorLogger, file.lang());
 
+        if (preprocessor.reportOutput(outputList, true))
+            return mLogger->exitcode();
+
         if (!preprocessor.loadFiles(files))
             return mLogger->exitcode();
 
@@ -1232,19 +1213,7 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
 
                 if (!hasValidConfig && currCfg == *configurations.rbegin()) {
                     // If there is no valid configuration then report error..
-                    std::string locfile = Path::fromNativeSeparators(o.location.file());
-                    if (mSettings.relativePaths)
-                        locfile = Path::getRelativePath(locfile, mSettings.basePaths);
-
-                    ErrorMessage::FileLocation loc1(locfile, o.location.line, o.location.col);
-
-                    ErrorMessage errmsg({std::move(loc1)},
-                                        file.spath(),
-                                        Severity::error,
-                                        o.msg,
-                                        "preprocessorErrorDirective",
-                                        Certainty::normal);
-                    mErrorLogger.reportErr(errmsg);
+                    preprocessor.error(o.location.file(), o.location.line, o.msg);
                 }
                 continue;
 

lib/preprocessor.cpp

@@ -899,7 +899,7 @@ void Preprocessor::error(const std::string &filename, unsigned int linenr, const
         if (mSettings.relativePaths)
             file = Path::getRelativePath(file, mSettings.basePaths);
 
-        locationList.emplace_back(file, linenr, 0);
+        locationList.emplace_back(file, linenr, 0); // TODO: set column
     }
     mErrorLogger.reportErr(ErrorMessage(std::move(locationList),
                                         mFile0,

lib/preprocessor.h

@@ -140,12 +140,13 @@ class CPPCHECKLIB WARN_UNUSED Preprocessor {
      */
     void dump(std::ostream &out) const;
 
-    static bool hasErrors(const simplecpp::Output &output);
-
-protected:
     bool reportOutput(const simplecpp::OutputList &outputList, bool showerror);
 
+    void error(const std::string &filename, unsigned int linenr, const std::string &msg);
+
 private:
+    static bool hasErrors(const simplecpp::Output &output);
+
     bool handleErrors(const simplecpp::OutputList &outputList, bool throwError);
 
     static void simplifyPragmaAsmPrivate(simplecpp::TokenList &tokenList);
@@ -159,7 +160,6 @@ class CPPCHECKLIB WARN_UNUSED Preprocessor {
     };
 
     void missingInclude(const std::string &filename, unsigned int linenr, unsigned int col, const std::string &header, HeaderTypes headerType);
-    void error(const std::string &filename, unsigned int linenr, const std::string &msg);
 
     void addRemarkComments(const simplecpp::TokenList &tokens, std::vector<RemarkComment> &remarkComments) const;
 
@@ -173,7 +173,7 @@ class CPPCHECKLIB WARN_UNUSED Preprocessor {
     simplecpp::FileDataCache mFileCache;
 
     /** filename for cpp/c file - useful when reporting errors */
-    std::string mFile0;
+    std::string mFile0; // TODO: this is never set
     Standards::Language mLang{Standards::Language::None};
 
     /** simplecpp tracking info */

test/cli/other_test.py

@@ -3429,7 +3429,8 @@ def test_preprocess_enforced_cpp(tmp_path):  # #10989
     assert exitcode == 0, stdout if stdout else stderr
     assert stdout.splitlines() == []
     assert stderr.splitlines() == [
-        '{}:2:2: error: #error "err" [preprocessorErrorDirective]'.format(test_file)
+        # TODO: lacks column information
+        '{}:2:0: error: #error "err" [preprocessorErrorDirective]'.format(test_file)
     ]
 
 
@@ -3847,3 +3848,106 @@ def test_unmatched_file(tmp_path):  # #14248 / #14249
         f'{lib_file}:-1:0: information: Unmatched suppression: error6 [unmatchedSuppression]'
     ]
     assert ret == 0, stdout
+
+
+def test_simplecpp_warning(tmp_path):
+    test_file = tmp_path / 'test.c'
+    with open(test_file, "w") as f:
+        f.write(
+"""
+#define warning "warn msg"
+""")
+
+    args = [
+        '-q',
+        '--template=simple',
+        str(test_file)
+    ]
+
+    exitcode, stdout, stderr = cppcheck(args)
+    assert exitcode == 0, stdout
+    assert stdout.splitlines() == []
+    assert stderr.splitlines() == []
+
+
+def test_simplecpp_unhandled_char(tmp_path):
+    test_file = tmp_path / 'test.c'
+    with open(test_file, "w", encoding='utf-8') as f:
+        f.write(
+"""
+int 你=0;
+""")
+
+    args = [
+        '-q',
+        '--template=simple',
+        '--emit-duplicates',
+        str(test_file)
+    ]
+
+    exitcode, stdout, stderr = cppcheck(args)
+    assert exitcode == 0, stdout
+    assert stdout.splitlines() == []
+    assert stderr.splitlines() == [
+        # TODO: lacks column information
+        # TODO: should report another ID
+        '{}:2:0: error: The code contains unhandled character(s) (character code=228). Neither unicode nor extended ascii is supported. [preprocessorErrorDirective]'.format(test_file)
+    ]
+
+
+def test_simplecpp_include_nested_too_deeply(tmp_path):
+    test_file = tmp_path / 'test.c'
+    with open(test_file, "w") as f:
+        f.write('#include "test.h"')
+
+    test_h = tmp_path / 'test.h'
+    with open(test_h, "w") as f:
+        f.write('#include "test_0.h"')
+
+    for i in range(400):
+        test_h = tmp_path / f'test_{i}.h'
+        with open(test_h, "w") as f:
+            f.write('#include "test_{}.h"'.format(i+1))
+
+    args = [
+        '-q',
+        '--template=simple',
+        '--emit-duplicates',
+        str(test_file)
+    ]
+
+    exitcode, stdout, stderr = cppcheck(args)
+    assert exitcode == 0, stdout
+    assert stdout.splitlines() == []
+    test_h = tmp_path / 'test_398.h'
+    assert stderr.splitlines() == [
+        # TODO: should only report the error once
+        # TODO: should report another ID
+        # TODO: lacks column information
+        '{}:1:0: error: #include nested too deeply [preprocessorErrorDirective]'.format(test_h),
+        '{}:1:0: error: #include nested too deeply [preprocessorErrorDirective]'.format(test_h)
+    ]
+
+
+def test_simplecpp_syntax_error(tmp_path):
+    test_file = tmp_path / 'test.c'
+    with open(test_file, "w") as f:
+        f.write('#include ""')
+
+    args = [
+        '-q',
+        '--template=simple',
+        '--emit-duplicates',
+        str(test_file)
+    ]
+
+    exitcode, stdout, stderr = cppcheck(args)
+    assert exitcode == 0, stdout
+    assert stdout.splitlines() == []
+    assert stderr.splitlines() == [
+        # TODO: should only report the error once
+        # TODO: should report another ID
+        # TODO: lacks column information
+        '{}:1:0: error: No header in #include [preprocessorErrorDirective]'.format(test_file),
+        '{}:1:0: error: No header in #include [preprocessorErrorDirective]'.format(test_file)
+    ]

test/cli/project_test.py

@@ -59,9 +59,12 @@ def test_json_entry_file_not_found(tmpdir):
     with open(project_file, 'w') as f:
         f.write(json.dumps(compilation_db))
 
-    ret, _, stderr = cppcheck(["--project=" + str(project_file)])
+    ret, _, stderr = cppcheck([
+        '--template=simple',
+        "--project=" + str(project_file)
+    ])
     assert 0 == ret
-    assert f"{missing_file}:1:0: error: File is missing: {missing_file_posix} [syntaxError]\n\n^\n" == stderr
+    assert stderr == f"nofile:0:0: error: File is missing: {missing_file_posix} [preprocessorErrorDirective]\n"
 
 
 def test_json_no_arguments(tmpdir):

test/testpreprocessor.cpp

@@ -49,21 +49,12 @@ class TestPreprocessor : public TestFixture {
     TestPreprocessor() : TestFixture("TestPreprocessor") {}
 
 private:
-    class PreprocessorTest : public Preprocessor
-    {
-        friend class TestPreprocessor;
-    public:
-        PreprocessorTest(simplecpp::TokenList& tokens, const Settings& settings, ErrorLogger &errorLogger, Standards::Language lang)
-            : Preprocessor(tokens, settings, errorLogger, lang)
-        {}
-    };
-
     template<size_t size>
     std::string expandMacros(const char (&code)[size], ErrorLogger &errorLogger) const {
         simplecpp::OutputList outputList;
         std::vector<std::string> files;
         simplecpp::TokenList tokens1 = simplecpp::TokenList(code, files, "file.cpp", &outputList);
-        PreprocessorTest p(tokens1, settingsDefault, errorLogger, Path::identify(tokens1.getFiles()[0], false));
+        Preprocessor p(tokens1, settingsDefault, errorLogger, Path::identify(tokens1.getFiles()[0], false));
         simplecpp::TokenList tokens2 = p.preprocess("", files, true);
         (void)p.reportOutput(outputList, true);
         return tokens2.stringify();
@@ -128,7 +119,7 @@ class TestPreprocessor : public TestFixture {
 
         simplecpp::TokenList tokens(code, size, files, Path::simplifyPath(filename), &outputList);
         // TODO: we should be using the actual Preprocessor implementation
-        PreprocessorTest preprocessor(tokens, settings, errorlogger, Path::identify(tokens.getFiles()[0], false));
+        Preprocessor preprocessor(tokens, settings, errorlogger, Path::identify(tokens.getFiles()[0], false));
 
         // TODO: should be possible without a Preprocessor instance
         if (preprocessor.reportOutput(outputList, true))

test/testsuppressions.cpp

@@ -88,8 +88,8 @@ class TestSuppressions : public TestFixture {
         TEST_CASE(suppressingSyntaxErrorsFS); // #7076
         TEST_CASE(suppressingSyntaxErrorsInlineFiles); // #5917
         TEST_CASE(suppressingSyntaxErrorsInlineFS); // #5917
-        TEST_CASE(suppressingSyntaxErrorsWhileFileReadFiles); // PR #1333
-        TEST_CASE(suppressingSyntaxErrorsWhileFileReadFS); // PR #1333
+        TEST_CASE(suppressingSimplecppErrorsWhileFileReadFiles); // PR #1333
+        TEST_CASE(suppressingSimplecppErrorsWhileFileReadFS); // PR #1333
         TEST_CASE(symbol);
 
         TEST_CASE(unusedFunctionFiles);
@@ -1341,7 +1341,7 @@ class TestSuppressions : public TestFixture {
         suppressingSyntaxErrorsInlineInternal(&TestSuppressions::checkSuppressionFS);
     }
 
-    void suppressingSyntaxErrorsWhileFileReadInternal(unsigned int (TestSuppressions::*check)(const char[], const std::string &)) { // syntaxError while file read should be suppressible (PR #1333)
+    void suppressingSimplecppErrorsWhileFileReadInternal(unsigned int (TestSuppressions::*check)(const char[], const std::string &)) { // syntaxError while file read should be suppressible (PR #1333)
         const char code[] = "CONST (genType, KS_CONST) genService[KS_CFG_NR_OF_NVM_BLOCKS] =\n"
                             "{\n"
                             "[!VAR \"BC\" = \"$BC + 1\"!][!//\n"
@@ -1355,16 +1355,16 @@ class TestSuppressions : public TestFixture {
                             "[!VAR \"BC\" = \"$BC + 1\"!][!//\n"
                             "[!ENDIF!][!//\n"
                             "};";
-        ASSERT_EQUALS(0, (this->*check)(code, "syntaxError:test.cpp:4"));
+        ASSERT_EQUALS(0, (this->*check)(code, "preprocessorErrorDirective:test.cpp:4"));
         ASSERT_EQUALS("", errout_str());
     }
 
-    void suppressingSyntaxErrorsWhileFileReadFiles() {
-        suppressingSyntaxErrorsWhileFileReadInternal(&TestSuppressions::checkSuppressionFiles);
+    void suppressingSimplecppErrorsWhileFileReadFiles() {
+        suppressingSimplecppErrorsWhileFileReadInternal(&TestSuppressions::checkSuppressionFiles);
     }
 
-    void suppressingSyntaxErrorsWhileFileReadFS() {
-        suppressingSyntaxErrorsWhileFileReadInternal(&TestSuppressions::checkSuppressionFiles);
+    void suppressingSimplecppErrorsWhileFileReadFS() {
+        suppressingSimplecppErrorsWhileFileReadInternal(&TestSuppressions::checkSuppressionFiles);
     }
 
     // TODO: this tests an internal function - should it be private?