[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 &Result) 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;", &Errors); - 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 &Result) 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;", &Errors); + 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
[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 &Result) 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;", &Errors); - 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 &Result) 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;", &Errors); + 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