[PATCH] D92756: [clang-tidy] Support all YAML supported spellings for bools in CheckOptions.

[Nathan James via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG68e642cad024: [clang-tidy] Support all YAML supported 
spellings for bools in CheckOptions. (authored by njames93).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92756/new/

https://reviews.llvm.org/D92756

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp


Index: clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -195,8 +195,9 @@
   CheckOptions["test.BoolIFalseValue"] = "0";
   CheckOptions["test.BoolTrueValue"] = "true";
   CheckOptions["test.BoolFalseValue"] = "false";
+  CheckOptions["test.BoolTrueShort"] = "Y";
+  CheckOptions["test.BoolFalseShort"] = "N";
   CheckOptions["test.BoolUnparseable"] = "Nothing";
-  CheckOptions["test.BoolCaseMismatch"] = "True";
 
   ClangTidyContext Context(std::make_unique(
   ClangTidyGlobalOptions(), Options));
@@ -227,12 +228,11 @@
   CHECK_VAL(TestCheck.getIntLocal("BoolIFalseValue"), false);
   CHECK_VAL(TestCheck.getIntLocal("BoolTrueValue"), true);
   CHECK_VAL(TestCheck.getIntLocal("BoolFalseValue"), false);
+  CHECK_VAL(TestCheck.getIntLocal("BoolTrueShort"), true);
+  CHECK_VAL(TestCheck.getIntLocal("BoolFalseShort"), false);
   CHECK_ERROR_INT(TestCheck.getIntLocal("BoolUnparseable"),
   "invalid configuration value 'Nothing' for option "
   "'test.BoolUnparseable'; expected a bool");
-  CHECK_ERROR_INT(TestCheck.getIntLocal("BoolCaseMismatch"),
-  "invalid configuration value 'True' for option "
-  "'test.BoolCaseMismatch'; expected a bool");
 
 #undef CHECK_ERROR_INT
 }
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -73,6 +73,9 @@
   ` and
   :doc:`modernize-make-unique `.
 
+- CheckOptions that take boolean values now support all spellings supported in 
+  the `YAML format `_.
+
 New modules
 ^^^
 
Index: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
@@ -11,6 +11,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/WithColor.h"
+#include "llvm/Support/YAMLParser.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace clang {
@@ -108,13 +109,14 @@
 
 static llvm::Expected getAsBool(StringRef Value,
   const llvm::Twine ) {
-  if (Value == "true")
-return true;
-  if (Value == "false")
-return false;
-  bool Result;
-  if (!Value.getAsInteger(10, Result))
-return Result;
+
+  if (llvm::Optional Parsed = llvm::yaml::parseBool(Value))
+return *Parsed;
+  // To maintain backwards compatability, we support parsing numbers as
+  // booleans, even though its not supported in YAML.
+  long long Number;
+  if (!Value.getAsInteger(10, Number))
+return Number != 0;
   return llvm::make_error(LookupName.str(),
  Value.str(), true);
 }


Index: clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -195,8 +195,9 @@
   CheckOptions["test.BoolIFalseValue"] = "0";
   CheckOptions["test.BoolTrueValue"] = "true";
   CheckOptions["test.BoolFalseValue"] = "false";
+  CheckOptions["test.BoolTrueShort"] = "Y";
+  CheckOptions["test.BoolFalseShort"] = "N";
   CheckOptions["test.BoolUnparseable"] = "Nothing";
-  CheckOptions["test.BoolCaseMismatch"] = "True";
 
   ClangTidyContext Context(std::make_unique(
   ClangTidyGlobalOptions(), Options));
@@ -227,12 +228,11 @@
   CHECK_VAL(TestCheck.getIntLocal("BoolIFalseValue"), false);
   CHECK_VAL(TestCheck.getIntLocal("BoolTrueValue"), true);
   CHECK_VAL(TestCheck.getIntLocal("BoolFalseValue"), false);
+  CHECK_VAL(TestCheck.getIntLocal("BoolTrueShort"), true);
+  CHECK_VAL(TestCheck.getIntLocal("BoolFalseShort"), false);
   CHECK_ERROR_INT(TestCheck.getIntLocal("BoolUnparseable"),
   "invalid configuration value 'Nothing' for option "
   "'test.BoolUnparseable'; expected a bool");
-  CHECK_ERROR_INT(TestCheck.getIntLocal("BoolCaseMismatch"),
-  "invalid 

[PATCH] D92756: [clang-tidy] Support all YAML supported spellings for bools in CheckOptions.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92756/new/

https://reviews.llvm.org/D92756

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92756: [clang-tidy] Support all YAML supported spellings for bools in CheckOptions.

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 311388.
njames93 added a comment.

Fix test failures and added some to demonstrate this behaviour. Haven't added 
exhaustive cases as they are present in the YAML parser tests.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92756/new/

https://reviews.llvm.org/D92756

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp


Index: clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -195,8 +195,9 @@
   CheckOptions["test.BoolIFalseValue"] = "0";
   CheckOptions["test.BoolTrueValue"] = "true";
   CheckOptions["test.BoolFalseValue"] = "false";
+  CheckOptions["test.BoolTrueShort"] = "Y";
+  CheckOptions["test.BoolFalseShort"] = "N";
   CheckOptions["test.BoolUnparseable"] = "Nothing";
-  CheckOptions["test.BoolCaseMismatch"] = "True";
 
   ClangTidyContext Context(std::make_unique(
   ClangTidyGlobalOptions(), Options));
@@ -227,12 +228,11 @@
   CHECK_VAL(TestCheck.getIntLocal("BoolIFalseValue"), false);
   CHECK_VAL(TestCheck.getIntLocal("BoolTrueValue"), true);
   CHECK_VAL(TestCheck.getIntLocal("BoolFalseValue"), false);
+  CHECK_VAL(TestCheck.getIntLocal("BoolTrueShort"), true);
+  CHECK_VAL(TestCheck.getIntLocal("BoolFalseShort"), false);
   CHECK_ERROR_INT(TestCheck.getIntLocal("BoolUnparseable"),
   "invalid configuration value 'Nothing' for option "
   "'test.BoolUnparseable'; expected a bool");
-  CHECK_ERROR_INT(TestCheck.getIntLocal("BoolCaseMismatch"),
-  "invalid configuration value 'True' for option "
-  "'test.BoolCaseMismatch'; expected a bool");
 
 #undef CHECK_ERROR_INT
 }
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -73,6 +73,9 @@
   ` and
   :doc:`modernize-make-unique `.
 
+- CheckOptions that take boolean values now support all spellings supported in 
+  the `YAML format `_.
+
 New modules
 ^^^
 
Index: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
@@ -11,6 +11,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/WithColor.h"
+#include "llvm/Support/YAMLParser.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace clang {
@@ -108,13 +109,14 @@
 
 static llvm::Expected getAsBool(StringRef Value,
   const llvm::Twine ) {
-  if (Value == "true")
-return true;
-  if (Value == "false")
-return false;
-  bool Result;
-  if (!Value.getAsInteger(10, Result))
-return Result;
+
+  if (llvm::Optional Parsed = llvm::yaml::parseBool(Value))
+return *Parsed;
+  // To maintain backwards compatability, we support parsing numbers as
+  // booleans, even though its not supported in YAML.
+  long long Number;
+  if (!Value.getAsInteger(10, Number))
+return Number != 0;
   return llvm::make_error(LookupName.str(),
  Value.str(), true);
 }


Index: clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -195,8 +195,9 @@
   CheckOptions["test.BoolIFalseValue"] = "0";
   CheckOptions["test.BoolTrueValue"] = "true";
   CheckOptions["test.BoolFalseValue"] = "false";
+  CheckOptions["test.BoolTrueShort"] = "Y";
+  CheckOptions["test.BoolFalseShort"] = "N";
   CheckOptions["test.BoolUnparseable"] = "Nothing";
-  CheckOptions["test.BoolCaseMismatch"] = "True";
 
   ClangTidyContext Context(std::make_unique(
   ClangTidyGlobalOptions(), Options));
@@ -227,12 +228,11 @@
   CHECK_VAL(TestCheck.getIntLocal("BoolIFalseValue"), false);
   CHECK_VAL(TestCheck.getIntLocal("BoolTrueValue"), true);
   CHECK_VAL(TestCheck.getIntLocal("BoolFalseValue"), false);
+  CHECK_VAL(TestCheck.getIntLocal("BoolTrueShort"), true);
+  CHECK_VAL(TestCheck.getIntLocal("BoolFalseShort"), false);
   CHECK_ERROR_INT(TestCheck.getIntLocal("BoolUnparseable"),
   "invalid configuration value 'Nothing' for option "
   "'test.BoolUnparseable'; expected a bool");
-  CHECK_ERROR_INT(TestCheck.getIntLocal("BoolCaseMismatch"),
-  "invalid configuration value 'True' for option "
- 

[PATCH] D92756: [clang-tidy] Support all YAML supported spellings for bools in CheckOptions.

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 309985.
njames93 added a comment.

Refactored based on changes to parent.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92756/new/

https://reviews.llvm.org/D92756

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst


Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -73,6 +73,9 @@
   ` and
   :doc:`modernize-make-unique `.
 
+- CheckOptions that take boolean values now support all spellings supported in 
+  the `YAML format `_.
+
 New modules
 ^^^
 
Index: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
@@ -11,6 +11,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/WithColor.h"
+#include "llvm/Support/YAMLParser.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace clang {
@@ -95,13 +96,14 @@
 
 static llvm::Expected getAsBool(StringRef Value,
   const llvm::Twine ) {
-  if (Value == "true")
-return true;
-  if (Value == "false")
-return false;
-  bool Result;
-  if (!Value.getAsInteger(10, Result))
-return Result;
+
+  if (llvm::Optional Parsed = llvm::yaml::parseBool(Value))
+return *Parsed;
+  // To maintain backwards compatability, we support parsing numbers as
+  // booleans, even though its not supported in YAML.
+  long long Number;
+  if (!Value.getAsInteger(10, Number))
+return Number != 0;
   return llvm::make_error(LookupName.str(),
  Value.str(), true);
 }


Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -73,6 +73,9 @@
   ` and
   :doc:`modernize-make-unique `.
 
+- CheckOptions that take boolean values now support all spellings supported in 
+  the `YAML format `_.
+
 New modules
 ^^^
 
Index: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
@@ -11,6 +11,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/WithColor.h"
+#include "llvm/Support/YAMLParser.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace clang {
@@ -95,13 +96,14 @@
 
 static llvm::Expected getAsBool(StringRef Value,
   const llvm::Twine ) {
-  if (Value == "true")
-return true;
-  if (Value == "false")
-return false;
-  bool Result;
-  if (!Value.getAsInteger(10, Result))
-return Result;
+
+  if (llvm::Optional Parsed = llvm::yaml::parseBool(Value))
+return *Parsed;
+  // To maintain backwards compatability, we support parsing numbers as
+  // booleans, even though its not supported in YAML.
+  long long Number;
+  if (!Value.getAsInteger(10, Number))
+return Number != 0;
   return llvm::make_error(LookupName.str(),
  Value.str(), true);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92756: [clang-tidy] Support all YAML supported spellings for bools in CheckOptions.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Is there a way to add test coverage for this to demonstrate that we handle what 
YAML considers a Boolean and that we don't allow oddities?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92756/new/

https://reviews.llvm.org/D92756

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92756: [clang-tidy] Support all YAML supported spellings for bools in CheckOptions.

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added a reviewer: aaron.ballman.
Herald added a subscriber: xazax.hun.
njames93 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Depends on D92755 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92756

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst


Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -73,6 +73,9 @@
   ` and
   :doc:`modernize-make-unique `.
 
+- CheckOptions that take boolean values now support all spellings supported in 
+  the `YAML format `_.
+
 New modules
 ^^^
 
Index: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
@@ -11,6 +11,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/WithColor.h"
+#include "llvm/Support/YAMLParser.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace clang {
@@ -101,13 +102,11 @@
 
 static llvm::Expected getAsBool(StringRef Value,
   const llvm::Twine ) {
-  if (Value == "true")
-return true;
-  if (Value == "false")
-return false;
-  bool Result;
-  if (!Value.getAsInteger(10, Result))
-return Result;
+  // To maintain backwards compatability, we support parsing numbers as
+  // booleans, even though its not supported in YAML.
+  if (llvm::Optional Parsed =
+  llvm::yaml::parseBool(Value, /*AllowNumeric=*/true))
+return *Parsed;
   return llvm::make_error(LookupName.str(),
  Value.str(), true);
 }


Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -73,6 +73,9 @@
   ` and
   :doc:`modernize-make-unique `.
 
+- CheckOptions that take boolean values now support all spellings supported in 
+  the `YAML format `_.
+
 New modules
 ^^^
 
Index: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
@@ -11,6 +11,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/WithColor.h"
+#include "llvm/Support/YAMLParser.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace clang {
@@ -101,13 +102,11 @@
 
 static llvm::Expected getAsBool(StringRef Value,
   const llvm::Twine ) {
-  if (Value == "true")
-return true;
-  if (Value == "false")
-return false;
-  bool Result;
-  if (!Value.getAsInteger(10, Result))
-return Result;
+  // To maintain backwards compatability, we support parsing numbers as
+  // booleans, even though its not supported in YAML.
+  if (llvm::Optional Parsed =
+  llvm::yaml::parseBool(Value, /*AllowNumeric=*/true))
+return *Parsed;
   return llvm::make_error(LookupName.str(),
  Value.str(), true);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits