[PATCH] D30567: [clang-tidy] Fix treating non-space whitespaces in checks list.
curdeius added a comment. In https://reviews.llvm.org/D30567#708797, @alexfh wrote: > Do you have commit rights? Yes, I will commit this ASAP. https://reviews.llvm.org/D30567 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30567: [clang-tidy] Fix treating non-space whitespaces in checks list.
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG. Thanks! Do you have commit rights? https://reviews.llvm.org/D30567 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30567: [clang-tidy] Fix treating non-space whitespaces in checks list.
curdeius updated this revision to Diff 92772.
curdeius added a comment.
Trim spaces only everywhere. Fix test.
https://reviews.llvm.org/D30567
Files:
clang-tidy/ClangTidyDiagnosticConsumer.cpp
unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
Index: unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
===
--- unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
+++ unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
@@ -1,87 +1,94 @@
-#include "ClangTidy.h"
-#include "ClangTidyTest.h"
-#include "gtest/gtest.h"
-
-namespace clang {
-namespace tidy {
-namespace test {
-
-class TestCheck : public ClangTidyCheck {
-public:
- TestCheck(StringRef Name, ClangTidyContext *Context)
- : ClangTidyCheck(Name, Context) {}
- void registerMatchers(ast_matchers::MatchFinder *Finder) override {
-Finder->addMatcher(ast_matchers::varDecl().bind("var"), this);
- }
- void check(const ast_matchers::MatchFinder::MatchResult ) override {
-const auto *Var = Result.Nodes.getNodeAs("var");
-// Add diagnostics in the wrong order.
-diag(Var->getLocation(), "variable");
-diag(Var->getTypeSpecStartLoc(), "type specifier");
- }
-};
-
-TEST(ClangTidyDiagnosticConsumer, SortsErrors) {
- std::vector Errors;
- runCheckOnCode("int a;", );
- EXPECT_EQ(2ul, Errors.size());
- EXPECT_EQ("type specifier", Errors[0].Message.Message);
- EXPECT_EQ("variable", Errors[1].Message.Message);
-}
-
-TEST(GlobList, Empty) {
- GlobList Filter("");
-
- EXPECT_TRUE(Filter.contains(""));
- EXPECT_FALSE(Filter.contains("aaa"));
-}
-
-TEST(GlobList, Nothing) {
- GlobList Filter("-*");
-
- EXPECT_FALSE(Filter.contains(""));
- EXPECT_FALSE(Filter.contains("a"));
- EXPECT_FALSE(Filter.contains("-*"));
- EXPECT_FALSE(Filter.contains("-"));
- EXPECT_FALSE(Filter.contains("*"));
-}
-
-TEST(GlobList, Everything) {
- GlobList Filter("*");
-
- EXPECT_TRUE(Filter.contains(""));
- EXPECT_TRUE(Filter.contains(""));
- EXPECT_TRUE(Filter.contains("-*"));
- EXPECT_TRUE(Filter.contains("-"));
- EXPECT_TRUE(Filter.contains("*"));
-}
-
-TEST(GlobList, Simple) {
- GlobList Filter("aaa");
-
- EXPECT_TRUE(Filter.contains("aaa"));
- EXPECT_FALSE(Filter.contains(""));
- EXPECT_FALSE(Filter.contains("aa"));
- EXPECT_FALSE(Filter.contains(""));
- EXPECT_FALSE(Filter.contains("bbb"));
-}
-
-TEST(GlobList, Complex) {
- GlobList Filter("*,-a.*, -b.*, a.1.* ,-a.1.A.*,-..,-...,-..+,-*$, -*qwe* ");
-
- EXPECT_TRUE(Filter.contains("aaa"));
- EXPECT_TRUE(Filter.contains("qqq"));
- EXPECT_FALSE(Filter.contains("a."));
- EXPECT_FALSE(Filter.contains("a.b"));
- EXPECT_FALSE(Filter.contains("b."));
- EXPECT_FALSE(Filter.contains("b.b"));
- EXPECT_TRUE(Filter.contains("a.1.b"));
- EXPECT_FALSE(Filter.contains("a.1.A.a"));
- EXPECT_FALSE(Filter.contains("qwe"));
- EXPECT_FALSE(Filter.contains("asdfqweasdf"));
- EXPECT_TRUE(Filter.contains("asdfqwEasdf"));
-}
-
-} // namespace test
-} // namespace tidy
-} // namespace clang
+#include "ClangTidy.h"
+#include "ClangTidyTest.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace tidy {
+namespace test {
+
+class TestCheck : public ClangTidyCheck {
+public:
+ TestCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override {
+Finder->addMatcher(ast_matchers::varDecl().bind("var"), this);
+ }
+ void check(const ast_matchers::MatchFinder::MatchResult ) override {
+const auto *Var = Result.Nodes.getNodeAs("var");
+// Add diagnostics in the wrong order.
+diag(Var->getLocation(), "variable");
+diag(Var->getTypeSpecStartLoc(), "type specifier");
+ }
+};
+
+TEST(ClangTidyDiagnosticConsumer, SortsErrors) {
+ std::vector Errors;
+ runCheckOnCode("int a;", );
+ EXPECT_EQ(2ul, Errors.size());
+ EXPECT_EQ("type specifier", Errors[0].Message.Message);
+ EXPECT_EQ("variable", Errors[1].Message.Message);
+}
+
+TEST(GlobList, Empty) {
+ GlobList Filter("");
+
+ EXPECT_TRUE(Filter.contains(""));
+ EXPECT_FALSE(Filter.contains("aaa"));
+}
+
+TEST(GlobList, Nothing) {
+ GlobList Filter("-*");
+
+ EXPECT_FALSE(Filter.contains(""));
+ EXPECT_FALSE(Filter.contains("a"));
+ EXPECT_FALSE(Filter.contains("-*"));
+ EXPECT_FALSE(Filter.contains("-"));
+ EXPECT_FALSE(Filter.contains("*"));
+}
+
+TEST(GlobList, Everything) {
+ GlobList Filter("*");
+
+ EXPECT_TRUE(Filter.contains(""));
+ EXPECT_TRUE(Filter.contains(""));
+ EXPECT_TRUE(Filter.contains("-*"));
+ EXPECT_TRUE(Filter.contains("-"));
+ EXPECT_TRUE(Filter.contains("*"));
+}
+
+TEST(GlobList, Simple) {
+ GlobList Filter("aaa");
+
+ EXPECT_TRUE(Filter.contains("aaa"));
+ EXPECT_FALSE(Filter.contains(""));
+ EXPECT_FALSE(Filter.contains("aa"));
+ EXPECT_FALSE(Filter.contains(""));
+ EXPECT_FALSE(Filter.contains("bbb"));
+}
+
+TEST(GlobList, WhitespacesAtBegin) {
+ GlobList Filter("-*, a.b.*");
+
+
[PATCH] D30567: [clang-tidy] Fix treating non-space whitespaces in checks list.
alexfh added inline comments.
Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:133
+ StringRef UntrimmedGlob = GlobList.substr(0, GlobList.find(','));
+ StringRef Glob = UntrimmedGlob.trim();
+ GlobList = GlobList.substr(UntrimmedGlob.size() + 1);
s/trim()/trim(' ')/
https://reviews.llvm.org/D30567
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30567: [clang-tidy] Fix treating non-space whitespaces in checks list.
alexfh added a comment. There's one more trim() you missed. And the test needs to be updated (`s/\\n/ /`). https://reviews.llvm.org/D30567 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30567: [clang-tidy] Fix treating non-space whitespaces in checks list.
curdeius added a comment. Hi Alex, is it OK now? https://reviews.llvm.org/D30567 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30567: [clang-tidy] Fix treating non-space whitespaces in checks list.
curdeius updated this revision to Diff 92630.
curdeius added a comment.
Trim only spaces.
https://reviews.llvm.org/D30567
Files:
clang-tidy/ClangTidyDiagnosticConsumer.cpp
unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
Index: unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
===
--- unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
+++ unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
@@ -1,87 +1,94 @@
-#include "ClangTidy.h"
-#include "ClangTidyTest.h"
-#include "gtest/gtest.h"
-
-namespace clang {
-namespace tidy {
-namespace test {
-
-class TestCheck : public ClangTidyCheck {
-public:
- TestCheck(StringRef Name, ClangTidyContext *Context)
- : ClangTidyCheck(Name, Context) {}
- void registerMatchers(ast_matchers::MatchFinder *Finder) override {
-Finder->addMatcher(ast_matchers::varDecl().bind("var"), this);
- }
- void check(const ast_matchers::MatchFinder::MatchResult ) override {
-const auto *Var = Result.Nodes.getNodeAs("var");
-// Add diagnostics in the wrong order.
-diag(Var->getLocation(), "variable");
-diag(Var->getTypeSpecStartLoc(), "type specifier");
- }
-};
-
-TEST(ClangTidyDiagnosticConsumer, SortsErrors) {
- std::vector Errors;
- runCheckOnCode("int a;", );
- EXPECT_EQ(2ul, Errors.size());
- EXPECT_EQ("type specifier", Errors[0].Message.Message);
- EXPECT_EQ("variable", Errors[1].Message.Message);
-}
-
-TEST(GlobList, Empty) {
- GlobList Filter("");
-
- EXPECT_TRUE(Filter.contains(""));
- EXPECT_FALSE(Filter.contains("aaa"));
-}
-
-TEST(GlobList, Nothing) {
- GlobList Filter("-*");
-
- EXPECT_FALSE(Filter.contains(""));
- EXPECT_FALSE(Filter.contains("a"));
- EXPECT_FALSE(Filter.contains("-*"));
- EXPECT_FALSE(Filter.contains("-"));
- EXPECT_FALSE(Filter.contains("*"));
-}
-
-TEST(GlobList, Everything) {
- GlobList Filter("*");
-
- EXPECT_TRUE(Filter.contains(""));
- EXPECT_TRUE(Filter.contains(""));
- EXPECT_TRUE(Filter.contains("-*"));
- EXPECT_TRUE(Filter.contains("-"));
- EXPECT_TRUE(Filter.contains("*"));
-}
-
-TEST(GlobList, Simple) {
- GlobList Filter("aaa");
-
- EXPECT_TRUE(Filter.contains("aaa"));
- EXPECT_FALSE(Filter.contains(""));
- EXPECT_FALSE(Filter.contains("aa"));
- EXPECT_FALSE(Filter.contains(""));
- EXPECT_FALSE(Filter.contains("bbb"));
-}
-
-TEST(GlobList, Complex) {
- GlobList Filter("*,-a.*, -b.*, a.1.* ,-a.1.A.*,-..,-...,-..+,-*$, -*qwe* ");
-
- EXPECT_TRUE(Filter.contains("aaa"));
- EXPECT_TRUE(Filter.contains("qqq"));
- EXPECT_FALSE(Filter.contains("a."));
- EXPECT_FALSE(Filter.contains("a.b"));
- EXPECT_FALSE(Filter.contains("b."));
- EXPECT_FALSE(Filter.contains("b.b"));
- EXPECT_TRUE(Filter.contains("a.1.b"));
- EXPECT_FALSE(Filter.contains("a.1.A.a"));
- EXPECT_FALSE(Filter.contains("qwe"));
- EXPECT_FALSE(Filter.contains("asdfqweasdf"));
- EXPECT_TRUE(Filter.contains("asdfqwEasdf"));
-}
-
-} // namespace test
-} // namespace tidy
-} // namespace clang
+#include "ClangTidy.h"
+#include "ClangTidyTest.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace tidy {
+namespace test {
+
+class TestCheck : public ClangTidyCheck {
+public:
+ TestCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override {
+Finder->addMatcher(ast_matchers::varDecl().bind("var"), this);
+ }
+ void check(const ast_matchers::MatchFinder::MatchResult ) override {
+const auto *Var = Result.Nodes.getNodeAs("var");
+// Add diagnostics in the wrong order.
+diag(Var->getLocation(), "variable");
+diag(Var->getTypeSpecStartLoc(), "type specifier");
+ }
+};
+
+TEST(ClangTidyDiagnosticConsumer, SortsErrors) {
+ std::vector Errors;
+ runCheckOnCode("int a;", );
+ EXPECT_EQ(2ul, Errors.size());
+ EXPECT_EQ("type specifier", Errors[0].Message.Message);
+ EXPECT_EQ("variable", Errors[1].Message.Message);
+}
+
+TEST(GlobList, Empty) {
+ GlobList Filter("");
+
+ EXPECT_TRUE(Filter.contains(""));
+ EXPECT_FALSE(Filter.contains("aaa"));
+}
+
+TEST(GlobList, Nothing) {
+ GlobList Filter("-*");
+
+ EXPECT_FALSE(Filter.contains(""));
+ EXPECT_FALSE(Filter.contains("a"));
+ EXPECT_FALSE(Filter.contains("-*"));
+ EXPECT_FALSE(Filter.contains("-"));
+ EXPECT_FALSE(Filter.contains("*"));
+}
+
+TEST(GlobList, Everything) {
+ GlobList Filter("*");
+
+ EXPECT_TRUE(Filter.contains(""));
+ EXPECT_TRUE(Filter.contains(""));
+ EXPECT_TRUE(Filter.contains("-*"));
+ EXPECT_TRUE(Filter.contains("-"));
+ EXPECT_TRUE(Filter.contains("*"));
+}
+
+TEST(GlobList, Simple) {
+ GlobList Filter("aaa");
+
+ EXPECT_TRUE(Filter.contains("aaa"));
+ EXPECT_FALSE(Filter.contains(""));
+ EXPECT_FALSE(Filter.contains("aa"));
+ EXPECT_FALSE(Filter.contains(""));
+ EXPECT_FALSE(Filter.contains("bbb"));
+}
+
+TEST(GlobList, WhitespacesAtBegin) {
+ GlobList Filter("-*,\n a.b.*");
+
+
[PATCH] D30567: [clang-tidy] Fix treating non-space whitespaces in checks list.
alexfh added a comment.
In https://reviews.llvm.org/D30567#696436, @curdeius wrote:
> Hi Alex and sorry for the late reply.
>
> The main use case is a more readable `.clang-tidy` configuration checks.
> Before this correction one can use something like this:
>
> ---
> Checks: '
> ,*,
> ,-cert-dcl03-c,
> '
> ...
>
>
> It works, but is hardly comprehensible to a newbie (the strange use of
> addtional commas).
> Since the spaces are ignored (since a recent commit of yours) we can add
> spaces after the leading comma and hope that no user uses a tab...
>
> After applying this patch, we can just write (with tabs or spaces and as many
> newlines as we want - used for grouping for instance):
>
> ---
> Checks: '
> *,
> -cert-dcl03-c,
> '
> ...
>
>
> Additionaly, you can sometimes accidentally issue a tabulator on the command
> line and that's just nice to ignore it.
Currently, one can use YAML folded strings to split the list of checks to
multiple lines:
Checks: >
-*,
cert-dcl03-c
So it's not necessary to trim newlines. And tabs are forbidden in YAML:
http://www.yaml.org/faq.html. So I'd suggest to leave `.trim(' ')`. The rest of
the change looks good - thanks for catching the bug ;)
https://reviews.llvm.org/D30567
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30567: [clang-tidy] Fix treating non-space whitespaces in checks list.
curdeius added a comment. Hi Alex and sorry for the late reply. The main use case is a more readable `.clang-tidy` configuration checks. Before this correction one can use something like this: --- Checks: ' ,*, ,-cert-dcl03-c, ' ... It works, but is hardly comprehensible to a newbie (the strange use of addtional commas). Since the spaces are ignored (since a recent commit of yours) we can remove the leading comma and hope that no user uses a tab... After applying this patch, we can just write (with tabs or spaces and as many newlines as we want - used for grouping for instance): --- Checks: ' *, -cert-dcl03-c, ' ... Additionaly, you can sometimes accidentally issue a tabulator on the command line and that's just nice to ignore it. https://reviews.llvm.org/D30567 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30567: [clang-tidy] Fix treating non-space whitespaces in checks list.
alexfh added a comment. What's the practical use of newlines and tab characters in the glob list? https://reviews.llvm.org/D30567 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
