Skip to content
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.

Sign up for GitHub

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

fixed #13570 - PathMatch could no longer match a file after it tried to match a directory #7242

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fixed #13570 - PathMatch could no longer match a file after it tried to match a directory #7242

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions lib/pathmatch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ bool PathMatch::match(const std::string &path) const
std::string findpath = Path::fromNativeSeparators(path);
if (!mCaseSensitive)
strTolower(findpath);
std::string finddir;
if (!endsWith(findpath,'/'))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The negation could be avoided by switching if/else.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left the previous logic intact.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like finddir could even be const if a ternary was used...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get the intention but it does not really makes things more readable. It would be great if findpath could also be const.

finddir = removeFilename(findpath);
else
finddir = findpath;

const bool is_absolute = Path::isAbsolute(path);

Expand All @@ -54,19 +59,16 @@ bool PathMatch::match(const std::string &path) const

// Filtering directory name
if (endsWith(pathToMatch,'/')) {
if (!endsWith(findpath,'/'))
findpath = removeFilename(findpath);

if (pathToMatch.length() > findpath.length())
if (pathToMatch.length() > finddir.length())
continue;
// Match relative paths starting with mask
// -isrc matches src/foo.cpp
if (findpath.compare(0, pathToMatch.size(), pathToMatch) == 0)
if (finddir.compare(0, pathToMatch.size(), pathToMatch) == 0)
return true;
// Match only full directory name in middle or end of the path
// -isrc matches myproject/src/ but does not match
// myproject/srcfiles/ or myproject/mysrc/
if (findpath.find("/" + pathToMatch) != std::string::npos)
if (finddir.find("/" + pathToMatch) != std::string::npos)
return true;
}
// Filtering filename
Expand Down
28 changes: 27 additions & 1 deletion test/cli/other_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2784,4 +2784,30 @@ def test_addon_suppr_cli_file_line(tmp_path):

def test_addon_suppr_cli_absfile_line(tmp_path):
test_file = tmp_path / 'test.c'
__test_addon_suppr(tmp_path, ['--suppress=misra-c2012-2.3:{}:3'.format(test_file)])
__test_addon_suppr(tmp_path, ['--suppress=misra-c2012-2.3:{}:3'.format(test_file)])


def test_file_ignore_2(tmp_path): # #13570
tests_path = tmp_path / 'tests'
os.mkdir(tests_path)

lib_path = tmp_path / 'lib'
os.mkdir(lib_path)

test_file_1 = lib_path / 'test_1.c'
with open(test_file_1, 'wt'):
pass

args = [
'-itests',
'-itest_1.c',
'.'
]

exitcode, stdout, stderr = cppcheck(args, cwd=tmp_path)
assert exitcode == 1, stdout
assert stdout.splitlines() == [
'cppcheck: error: could not find or open any of the paths given.',
'cppcheck: Maybe all paths were ignored?'
]
assert stderr.splitlines() == []
19 changes: 19 additions & 0 deletions test/testpathmatch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ class TestPathMatch : public TestFixture {
TEST_CASE(onemasklongerpath1);
TEST_CASE(onemasklongerpath2);
TEST_CASE(onemasklongerpath3);
TEST_CASE(onemaskcwd);
TEST_CASE(twomasklongerpath1);
TEST_CASE(twomasklongerpath2);
TEST_CASE(twomasklongerpath3);
Expand All @@ -60,10 +61,12 @@ class TestPathMatch : public TestFixture {
TEST_CASE(filemaskdifferentcase);
TEST_CASE(filemask2);
TEST_CASE(filemask3);
TEST_CASE(filemaskcwd);
TEST_CASE(filemaskpath1);
TEST_CASE(filemaskpath2);
TEST_CASE(filemaskpath3);
TEST_CASE(filemaskpath4);
TEST_CASE(mixedallmatch);
}

// Test empty PathMatch
Expand Down Expand Up @@ -146,6 +149,10 @@ class TestPathMatch : public TestFixture {
ASSERT(srcMatcher.match("project/src/module/"));
}

void onemaskcwd() const {
ASSERT(!srcMatcher.match("./src"));
}

void twomasklongerpath1() const {
std::vector<std::string> masks = { "src/", "module/" };
PathMatch match(std::move(masks));
Expand Down Expand Up @@ -189,6 +196,10 @@ class TestPathMatch : public TestFixture {
ASSERT(fooCppMatcher.match("src/foo.cpp"));
}

void filemaskcwd() const {
ASSERT(fooCppMatcher.match("./lib/foo.cpp"));
}

// Test PathMatch containing "src/foo.cpp"
void filemaskpath1() const {
ASSERT(srcFooCppMatcher.match("src/foo.cpp"));
Expand All @@ -205,6 +216,14 @@ class TestPathMatch : public TestFixture {
void filemaskpath4() const {
ASSERT(!srcFooCppMatcher.match("bar/foo.cpp"));
}

void mixedallmatch() const { // #13570
// when trying to match a directory against a directory entry it erroneously modified a local variable also used for file matching
std::vector<std::string> masks = { "tests/", "file.c" };
PathMatch match(std::move(masks));
ASSERT(match.match("tests/"));
ASSERT(match.match("lib/file.c"));
}
};

REGISTER_TEST(TestPathMatch)
Loading