[PATCH] D19547: [clang-tidy] Add FixIt for swapping arguments in string-constructor-checker.

[Etienne Bergeron via cfe-commits]

etienneb added inline comments.

================
Comment at: clang-tidy/misc/StringConstructorCheck.cpp:110
@@ -106,2 +109,3 @@
   if (Result.Nodes.getNodeAs<Expr>("swapped-parameter")) {
-    diag(Loc, "constructor parameters are probably swapped");
+    assert(E->getNumArgs() == 2 && "Invalid number of arguments");
+    auto D = diag(Loc, "constructor parameters are probably swapped");
----------------
alexfh wrote:
> Assertions should verify logical bugs in the code, not some properties of the user input. The matcher doesn't seem to ensure that the argument count is always 2, so it's possible to create code that will trigger this assertion. Either add `argumentCountIs(2)` to the matcher or remove the assertion.
As I get it, it's not needed to add ' argumentCountIs(2)' because:

          hasArgument(0, hasType(CharPtrType)),
          hasArgument(1, hasType(isInteger())),

the code already validates that there is at least two parameters.
The assert would fail if there is more parameters, which can occur only on a incorrect definition of the constructor.

So, I'm i favor to just remove the assert.


http://reviews.llvm.org/D19547