Skip to content

Commit b78a8c0

Browse files
glankkglankk
authored
Fixup #14021 (Add file mode specifier to PathMatch) (danmar#7689)
This is an addendum to danmar#7645 that adds the ability to specify directory-only matching in PathMatch patterns with a trailing slash. I originally didn't add this because I wanted to keep PathMatch purely syntactic, and this feature seemed like it would require filesystem calls in the PathMatch code. This PR solves that problem by lifting the responsibility of file mode checking to the caller, thus keeping PathMatch purely syntactic while still supporting directory-only matching. Previously `/test/foo/` would match `/test/foo` even if `foo` is a regular file. With this change the caller can specify the file mode of the file named by the provided path, and `/test/foo` will only match when the file mode is directory. The semantics of patterns that do not have a trailing slash is unchanged, e.g. `/test/foo` still matches `/test/foo/` and `/test/foo/bar.cpp`, regardless of the file mode. Also adds some more tests.
1 parent a80812f commit b78a8c0

File tree

5 files changed

+99
-43
lines changed

5 files changed

+99
-43
lines changed

‎cli/filelister.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ static std::string addFiles2(std::list<FileWithDetails>&files, const std::string
129129
} else {
130130
// Directory
131131
if (recursive) {
132-
if (!ignored.match(fname)) {
132+
if (!ignored.match(fname, PathMatch::Filemode::directory)) {
133133
std::list<FileWithDetails> filesSorted;
134134

135135
std::string err = addFiles2(filesSorted, fname, extra, recursive, ignored);
@@ -241,7 +241,7 @@ static std::string addFiles2(std::list<FileWithDetails> &files,
241241
#endif
242242
if (path_is_directory) {
243243
if (recursive) {
244-
if (!ignored.match(new_path)) {
244+
if (!ignored.match(new_path, PathMatch::Filemode::directory)) {
245245
std::string err = addFiles2(files, new_path, extra, recursive, ignored, debug);
246246
if (!err.empty()) {
247247
return err;

‎lib/pathmatch.cpp‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,29 +30,44 @@ PathMatch::PathMatch(std::vector<std::string> patterns, std::string basepath, Sy
3030
mPatterns(std::move(patterns)), mBasepath(std::move(basepath)), mSyntax(syntax)
3131
{}
3232

33-
bool PathMatch::match(const std::string &path) const
33+
bool PathMatch::match(const std::string &path, Filemode mode) const
3434
{
3535
return std::any_of(mPatterns.cbegin(), mPatterns.cend(), [=] (const std::string &pattern) {
36-
return match(pattern, path, mBasepath, mSyntax);
36+
return match(pattern, path, mBasepath, mode, mSyntax);
3737
});
3838
}
3939

40-
bool PathMatch::match(const std::string &pattern, const std::string &path, const std::string &basepath, Syntax syntax)
40+
bool PathMatch::match(const std::string &pattern, const std::string &path, const std::string &basepath, Filemode mode, Syntax syntax)
4141
{
42+
/* Fast paths for common patterns */
4243
if (pattern.empty())
4344
return false;
4445

4546
if (pattern == "*" || pattern == "**")
4647
return true;
4748

48-
/* A "real" path is absolute or relative to the base path. A pattern that isn't "real" can match at any
49+
/* If the pattern ends with a path separator it matches only directories. If the path names a regular file then
50+
* the last path component can't match. */
51+
bool dir_mismatch = PathIterator::issep(pattern.back(), syntax) && mode != Filemode::directory;
52+
53+
if (!dir_mismatch && pattern == path)
54+
return true;
55+
56+
/* A "real" pattern is absolute or relative to the base path. A pattern that isn't real can match at any
4957
* path component boundary. */
5058
bool real = Path::isAbsolute(pattern) || isRelativePattern(pattern);
5159

5260
/* Pattern iterator */
5361
PathIterator s = PathIterator::fromPattern(pattern, basepath, syntax);
5462
/* Path iterator */
5563
PathIterator t = PathIterator::fromPath(path, basepath, syntax);
64+
65+
if (dir_mismatch) {
66+
/* Final compponent can't match, so skip it. */
67+
while (*t != '\0' && *t != '/')
68+
++t;
69+
}
70+
5671
/* Pattern restart position */
5772
PathIterator p = s;
5873
/* Path restart position */
@@ -72,8 +87,6 @@ bool PathMatch::match(const std::string &pattern, const std::string &path, const
7287
slash = true;
7388
++s;
7489
}
75-
/* Add backtrack for matching zero characters */
76-
b.emplace(s.getpos(), t.getpos());
7790
while (*t != '\0' && (slash || *t != '/')) {
7891
if (*s == *t) {
7992
/* Could stop here, but do greedy match and add

‎lib/pathmatch.h‎

Lines changed: 35 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,11 @@
5454
* - Otherwise:
5555
* - Match all files where the pattern matches any part of the file's canonical absolute path up until a
5656
* path separator or the end of the pathname, and the matching part directly follows a path separator.
57+
* - If a pattern ends with a path separator before canonicalization then the final path component of the pattern
58+
* only matches the final path component of the file path if the file is a directory (specified by the file mode
59+
* parameter).
5760
*
58-
* TODO: Handle less common windows windows syntaxes:
61+
* TODO: Handle less common windows syntaxes:
5962
* - Drive-specific relative path: C:dir\foo.cpp
6063
* - Root-relative path: \dir\foo.cpp
6164
**/
@@ -87,6 +90,17 @@ class CPPCHECKLIB PathMatch {
8790
static constexpr Syntax platform_syntax = Syntax::unix;
8891
#endif
8992

93+
/**
94+
* @brief File mode of a file being matched
95+
*
96+
* regular: File is a regular file.
97+
* directory: File is a directory.
98+
*/
99+
enum class Filemode : std::uint8_t {
100+
regular,
101+
directory,
102+
};
103+
90104
/**
91105
* The constructor.
92106
*
@@ -100,20 +114,22 @@ class CPPCHECKLIB PathMatch {
100114
* @brief Match path against list of patterns.
101115
*
102116
* @param path Path to match.
117+
* @param mode The file mode of the file named by the path.
103118
* @return true if any of the masks match the path, false otherwise.
104119
*/
105-
bool match(const std::string &path) const;
120+
bool match(const std::string &path, Filemode mode = Filemode::regular) const;
106121

107122
/**
108123
* @brief Match path against a single pattern.
109124
*
110125
* @param pattern Pattern to use.
111126
* @param path Path to match.
112127
* @param basepath Path to which the pattern and path is relative, when applicable.
128+
* @param mode The file mode of the file named by the path.
113129
* @param syntax Path syntax.
114130
* @return true if the pattern matches the path, false otherwise.
115131
*/
116-
static bool match(const std::string &pattern, const std::string &path, const std::string &basepath = std::string(), Syntax syntax = platform_syntax);
132+
static bool match(const std::string &pattern, const std::string &path, const std::string &basepath = std::string(), Filemode mode = Filemode::regular, Syntax syntax = platform_syntax);
117133

118134
/**
119135
* @brief Check if a pattern is a relative path name.
@@ -209,12 +225,6 @@ class PathMatch::PathIterator {
209225
explicit PathIterator(const char *path_a = nullptr, const char *path_b = nullptr, Syntax syntax = platform_syntax) :
210226
mStart{path_a, path_b}, mSyntax(syntax)
211227
{
212-
const auto issep = [syntax] (char c) {
213-
return c == '/' || (syntax == Syntax::windows && c == '\\');
214-
};
215-
const auto isdrive = [] (char c) {
216-
return (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z');
217-
};
218228

219229
for (int i = 0; i < 2; i++) {
220230
const char *&p = mEnd[i];
@@ -225,19 +235,19 @@ class PathMatch::PathIterator {
225235

226236
if (mPos.l == 0) {
227237
/* Check length of root component */
228-
if (issep(p[0])) {
238+
if (issep(p[0], syntax)) {
229239
mRootLength++;
230-
if (syntax == Syntax::windows && issep(p[1])) {
240+
if (syntax == Syntax::windows && issep(p[1], syntax)) {
231241
mRootLength++;
232242
if (p[2] == '.' || p[2] == '?') {
233243
mRootLength++;
234-
if (issep(p[3]))
244+
if (issep(p[3], syntax))
235245
mRootLength++;
236246
}
237247
}
238248
} else if (syntax == Syntax::windows && isdrive(p[0]) && p[1] == ':') {
239249
mRootLength += 2;
240-
if (issep(p[2]))
250+
if (issep(p[2], syntax))
241251
mRootLength++;
242252
}
243253
p += mRootLength;
@@ -308,6 +318,18 @@ class PathMatch::PathIterator {
308318
return str;
309319
}
310320

321+
/* Syntax helper, check if a character is a path separator */
322+
static bool issep(char c, Syntax syntax)
323+
{
324+
return c == '/' || (syntax == Syntax::windows && c == '\\');
325+
}
326+
327+
/* Syntax helper, check if a chracter is a drive letter */
328+
static bool isdrive(char c)
329+
{
330+
return (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z');
331+
}
332+
311333
private:
312334
/* Read the current character */
313335
char current() const

‎lib/suppressions.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,7 @@ SuppressionList::Suppression::Result SuppressionList::Suppression::isSuppressed(
401401
if (!thisAndNextLine || lineNumber + 1 != errmsg.lineNumber)
402402
return Result::None;
403403
}
404-
if (!fileName.empty() && fileName != errmsg.getFileName() && !PathMatch::match(fileName, errmsg.getFileName()))
404+
if (!fileName.empty() && !PathMatch::match(fileName, errmsg.getFileName()))
405405
return Result::None;
406406
if (hash > 0 && hash != errmsg.hash)
407407
return Result::Checked;

‎test/testpathmatch.cpp‎

Lines changed: 42 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ class TestPathMatch : public TestFixture {
3030
private:
3131
static constexpr auto unix = PathMatch::Syntax::unix;
3232
static constexpr auto windows = PathMatch::Syntax::windows;
33+
static constexpr auto ifreg = PathMatch::Filemode::regular;
34+
static constexpr auto ifdir = PathMatch::Filemode::directory;
3335
#ifdef _WIN32
3436
const std::string basepath{"C:\\test"};
3537
#else
@@ -73,7 +75,8 @@ class TestPathMatch : public TestFixture {
7375
TEST_CASE(filemaskpath3);
7476
TEST_CASE(filemaskpath4);
7577
TEST_CASE(mixedallmatch);
76-
TEST_CASE(glob);
78+
TEST_CASE(glob1);
79+
TEST_CASE(glob2);
7780
TEST_CASE(globstar1);
7881
TEST_CASE(globstar2);
7982
TEST_CASE(pathiterator);
@@ -85,16 +88,16 @@ class TestPathMatch : public TestFixture {
8588
}
8689

8790
void emptymaskpath1() const {
88-
ASSERT(!emptyMatcher.match("src/"));
91+
ASSERT(!emptyMatcher.match("src/", ifdir));
8992
}
9093

9194
void emptymaskpath2() const {
92-
ASSERT(!emptyMatcher.match("../src/"));
95+
ASSERT(!emptyMatcher.match("../src/", ifdir));
9396
}
9497

9598
void emptymaskpath3() const {
96-
ASSERT(!emptyMatcher.match("/home/user/code/src/"));
97-
ASSERT(!emptyMatcher.match("d:/home/user/code/src/"));
99+
ASSERT(!emptyMatcher.match("/home/user/code/src/", ifdir));
100+
ASSERT(!emptyMatcher.match("d:/home/user/code/src/", ifdir));
98101
}
99102

100103
// Test PathMatch containing "src/"
@@ -103,17 +106,20 @@ class TestPathMatch : public TestFixture {
103106
}
104107

105108
void onemasksamepath() const {
106-
ASSERT(srcMatcher.match("src/"));
109+
ASSERT(srcMatcher.match("src/", ifdir));
110+
ASSERT(!srcMatcher.match("src/", ifreg));
107111
}
108112

109113
void onemasksamepathdifferentslash() const {
110114
PathMatch srcMatcher2({"src\\"}, basepath, windows);
111-
ASSERT(srcMatcher2.match("src/"));
115+
ASSERT(srcMatcher2.match("src/", ifdir));
116+
ASSERT(!srcMatcher2.match("src/", ifreg));
112117
}
113118

114119
void onemasksamepathdifferentcase() const {
115120
PathMatch match({"sRc/"}, basepath, windows);
116-
ASSERT(match.match("srC/"));
121+
ASSERT(match.match("srC/", ifdir));
122+
ASSERT(!match.match("srC/", ifreg));
117123
}
118124

119125
void onemasksamepathwithfile() const {
@@ -125,8 +131,8 @@ class TestPathMatch : public TestFixture {
125131
const std::string shorterToMatch("src/");
126132
ASSERT(shorterToMatch.length() < longerExclude.length());
127133
PathMatch match({longerExclude});
128-
ASSERT(match.match(longerExclude));
129-
ASSERT(!match.match(shorterToMatch));
134+
ASSERT(match.match(longerExclude, ifdir));
135+
ASSERT(!match.match(shorterToMatch, ifdir));
130136
}
131137

132138
void onemaskdifferentdir1() const {
@@ -146,40 +152,43 @@ class TestPathMatch : public TestFixture {
146152
}
147153

148154
void onemasklongerpath1() const {
149-
ASSERT(srcMatcher.match("/tmp/src/"));
150-
ASSERT(srcMatcher.match("d:/tmp/src/"));
155+
ASSERT(srcMatcher.match("/tmp/src/", ifdir));
156+
ASSERT(srcMatcher.match("d:/tmp/src/", ifdir));
151157
}
152158

153159
void onemasklongerpath2() const {
154-
ASSERT(srcMatcher.match("src/module/"));
160+
ASSERT(srcMatcher.match("src/module/", ifdir));
155161
}
156162

157163
void onemasklongerpath3() const {
158-
ASSERT(srcMatcher.match("project/src/module/"));
164+
ASSERT(srcMatcher.match("project/src/module/", ifdir));
159165
}
160166

161167
void onemaskcwd() const {
162-
ASSERT(srcMatcher.match("./src"));
168+
ASSERT(srcMatcher.match("./src", ifdir));
163169
}
164170

165171
void twomasklongerpath1() const {
166172
PathMatch match({ "src/", "module/" });
167-
ASSERT(!match.match("project/"));
173+
ASSERT(!match.match("project/", ifdir));
168174
}
169175

170176
void twomasklongerpath2() const {
171177
PathMatch match({ "src/", "module/" });
172-
ASSERT(match.match("project/src/"));
178+
ASSERT(match.match("project/src/", ifdir));
179+
ASSERT(!match.match("project/src/", ifreg));
173180
}
174181

175182
void twomasklongerpath3() const {
176183
PathMatch match({ "src/", "module/" });
177-
ASSERT(match.match("project/module/"));
184+
ASSERT(match.match("project/module/", ifdir));
185+
ASSERT(!match.match("project/module/", ifreg));
178186
}
179187

180188
void twomasklongerpath4() const {
181189
PathMatch match({ "src/", "module/" });
182-
ASSERT(match.match("project/src/module/"));
190+
ASSERT(match.match("project/src/module/", ifdir));
191+
ASSERT(match.match("project/src/module/", ifreg));
183192
}
184193

185194
// Test PathMatch containing "foo.cpp"
@@ -224,11 +233,12 @@ class TestPathMatch : public TestFixture {
224233
void mixedallmatch() const { // #13570
225234
// when trying to match a directory against a directory entry it erroneously modified a local variable also used for file matching
226235
PathMatch match({ "tests/", "file.c" });
227-
ASSERT(match.match("tests/"));
236+
ASSERT(match.match("tests/", ifdir));
237+
ASSERT(!match.match("tests/", ifreg));
228238
ASSERT(match.match("lib/file.c"));
229239
}
230240

231-
void glob() const {
241+
void glob1() const {
232242
PathMatch match({"test?.cpp"});
233243
ASSERT(match.match("test1.cpp"));
234244
ASSERT(match.match("src/test1.cpp"));
@@ -237,12 +247,22 @@ class TestPathMatch : public TestFixture {
237247
ASSERT(!match.match("test.cpp"));
238248
}
239249

250+
void glob2() const {
251+
PathMatch match({"test*.cpp"});
252+
ASSERT(match.match("test1.cpp"));
253+
ASSERT(match.match("src/test1.cpp"));
254+
ASSERT(match.match("test1.cpp/src"));
255+
ASSERT(!match.match("test1.c"));
256+
ASSERT(match.match("test.cpp"));
257+
}
258+
240259
void globstar1() const {
241260
PathMatch match({"src/**/foo.c"});
242261
ASSERT(match.match("src/lib/foo/foo.c"));
243262
ASSERT(match.match("src/lib/foo/bar/foo.c"));
244263
ASSERT(!match.match("src/lib/foo/foo.cpp"));
245264
ASSERT(!match.match("src/lib/foo/bar/foo.cpp"));
265+
ASSERT(!match.match("src/foo.c"));
246266
}
247267

248268
void globstar2() const {
@@ -251,6 +271,7 @@ class TestPathMatch : public TestFixture {
251271
ASSERT(match.match("src/lib/foo/bar/foo.c"));
252272
ASSERT(!match.match("src/lib/foo/foo.cpp"));
253273
ASSERT(!match.match("src/lib/foo/bar/foo.cpp"));
274+
ASSERT(!match.match("src/foo.c"));
254275
}
255276

256277
void pathiterator() const {

0 commit comments

Comments
 (0)