-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Partial fix for #11797: If sourcefile in imported project is missing then report critical error fileNotFound #5498
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -659,8 +659,23 @@ unsigned int CppCheck::check(const std::string &path, const std::string &content | |
return checkFile(Path::simplifyPath(path), emptyString, &iss); | ||
} | ||
|
||
void CppCheck::fileNotFoundError(const std::string& filename) | ||
{ | ||
mErrorLogger.reportErr(ErrorMessage({ErrorMessage::FileLocation(filename, 0, 0)}, | ||
filename, | ||
Severity::error, | ||
"File not found", | ||
"fileNotFound", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The error might be too generic. Something more specific would be better since there might be more errors like this coming soon. |
||
Certainty::normal)); | ||
mExitCode = 1; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should not arbitrary set the exitcode. |
||
} | ||
|
||
unsigned int CppCheck::check(const ImportProject::FileSettings &fs) | ||
{ | ||
if (!Path::isFile(fs.filename)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Other projects perform these checks with I also think this might be the wrong location to do this. I think this should be reported closer to the actual import but I think we might be lacking common code to place this in. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can put it in importproject also. But I wanted to achieve that the analysis will continue on other files that are found. I saw that if a file is missing when compile_commands.json is loaded then all analysis is aborted and that might be unfortunate. And putting this check here means that if the file has been removed after the project was loaded then we still detect this problem and bails out with an error message. Still.. maybe we can tune this and report it if simplecpp fails to read the source file instead. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I understood the intention. I just want it to be consistent in implementation and test. We should do in in the import step as we do with The problem is that we are not able to collect errors there - yet. That's what I added So this change is fine for now as we cannot solve it in a better way yet. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will post a draft PR soon after my pending stuff has been merged and I finished up most of the executor stuff. If things go well that should be as soon as next week. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
hmm yes
ok sure.
I would like to have this in next major release 2.13 so well it's not a hurry yet. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Simply adding tests with missing files for all projects would be sufficient. |
||
fileNotFoundError(fs.filename); | ||
return mExitCode; | ||
} | ||
CppCheck temp(mErrorLogger, mUseGlobalSuppressions, mExecuteCommand); | ||
temp.mSettings = mSettings; | ||
if (!temp.mSettings.userDefines.empty()) | ||
|
@@ -1715,6 +1730,7 @@ void CppCheck::getErrorMessages(ErrorLogger &errorlogger) | |
cppcheck.purgedConfigurationMessage(emptyString,emptyString); | ||
cppcheck.mTooManyConfigs = true; | ||
cppcheck.tooManyConfigsError(emptyString,0U); | ||
cppcheck.fileNotFoundError("missing.c"); | ||
// TODO: add functions to get remaining error messages | ||
|
||
// call all "getErrorMessages" in all registered Check classes | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -153,3 +153,13 @@ def test_gui_project_loads_absolute_vs_solution_2(): | |
ret, stdout, stderr = cppcheck(['--project=test.cppcheck']) | ||
assert ret == 0, stdout | ||
assert stderr == ERR_A + ERR_B | ||
|
||
def test_vs_project_missing_file(): | ||
with open('proj2/proj2.vcxproj', 'rt') as fin: | ||
s = fin.read() | ||
with open('proj2/test.vcxproj', 'wt') as fout: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should use |
||
fout.write(s.replace('a.c', 'a_missing.c')) | ||
ret, stdout, stderr = cppcheck(['--project=proj2/test.vcxproj', '--file-filter=*/a*'], status_report=True) | ||
assert 'proj2/a/a_missing.c:0:0: error: File not found [fileNotFound]' in stderr | ||
assert 'There was critical errors' in stdout | ||
os.remove('proj2/test.vcxproj') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error might be too generic. Something more specific would be better since there might be more errors like this coming soon.