Add an AST matcher for string-literal length

[etienneb]

Summary

This patch is adding support for a matcher to check string literal length.

This matcher is used in clang-tidy checkers and is part of this refactoring:

see: http://reviews.llvm.org/D19841

[etienneb]

fix doc

[aaron.ballman]

include/clang/ASTMatchers/ASTMatchers.h

1576

Split these onto two lines?

1579

Perhaps we can adjust the hasSize() matcher instead? It currently works with ConstantArrayType, but it seems reasonable for it to also work with StringLiteral.

[etienneb]

include/clang/ASTMatchers/ASTMatchers.h

1579

I didn't like the term "size" as it typically refer to the size in bytes.
Which is not the same for a wide-string.

Now, there is two different convention for naming matchers:

hasLength   and  lengthIs  ?

Any toughs on that?

[etienneb]

Aaron? could you comment on it?

include/clang/ASTMatchers/ASTMatchers.h

1576

If I look around, it seems to be more consistent to keep it on the same line (line 1563)

///   char *s = "abcd"; wchar_t *ws = L"abcd";

///   int array[4] = {1}; vector int myvec = (vector int)(1, 2);

///   char ch = 'a'; wchar_t chw = L'a';
``

1579

Here is the matcher for hasSize

AST_MATCHER_P(ConstantArrayType, hasSize, unsigned, N) {
  return Node.getSize() == N;
}

It's getting the getSize attribute. I believe we should stick with the name of the attribute.
But, I'm not sure if we should use hasLength, or lengthIs.

[aaron.ballman]

include/clang/ASTMatchers/ASTMatchers.h

1576

I don't have a strong opinion on it; however, since these get turned into examples that are on the website, I would weakly prefer the examples not be hideous. :-P

1579

I'm not too worried about size vs length (for instance, std::string has both). I would imagine this being implemented the same way we do other things with variance in API but not concept. See GetBodyMatcher in ASTMatchersInternal.h (and others near there) as an example.

I prefer hasSize because the two concepts are quite similar. For instance, a string literal's type is of a constant array type already.

[etienneb]

Other opinions?
I'll proceed to the cleanup if no one else has comments.

include/clang/ASTMatchers/ASTMatchers.h

1576

Ditto. No strong opinion.
But, I like consistency. I'm willing to fix all other instances too.

1579

I do not have strong opinion too on the naming. I'm curious if others also has opinion on it?
Then, I'll proceed.

[etienneb]

include/clang/ASTMatchers/ASTMatchers.h

1579

For instance, a string literal's type

stringLiteral is matching an expression and not a type.
And, hasSize is currently applied on types.
Do we want this?

[etienneb]

merging hasSize

[etienneb]

fix unittests

[etienneb]

Aaron? minus re-generation of the doc.
Is that what you want to see?

note: returned types for both getLength are not the same (APInt vs unsigned int).
I took the 'hasSize'-Matcher approach to let overloaded operators do the job for dispatching types.

note: there is a unittest for top-level node that is no longer valid because stringLiteral can be a top-level node now.

[aaron.ballman]

In D19876#423582, @etienneb wrote:

Aaron? minus re-generation of the doc.
Is that what you want to see?

Yes, this is the direction I was hoping for, thank you!

note: returned types for both getLength are not the same (APInt vs unsigned int).
I took the 'hasSize'-Matcher approach to let overloaded operators do the job for dispatching types.

Perfect!

note: there is a unittest for top-level node that is no longer valid because stringLiteral can be a top-level node now.

Thank you for that explanation, I was curious about that. :-)

[etienneb]

re-generating doc

[aaron.ballman]

LGTM, thank you for this!

include/clang/ASTMatchers/ASTMatchers.h

 const internal::VariadicDynCastAllOfMatcher<
   Stmt,
   CXXBoolLiteralExpr> cxxBoolLiteral;
 
 /// \brief Matches string literals (also matches wide string literals).
 ///
 /// Example matches "abcd", L"abcd"
 /// \code
-///   char *s = "abcd"; wchar_t *ws = L"abcd"
+///   char *s = "abcd";
+///   wchar_t *ws = L"abcd";
 /// \endcode
 const internal::VariadicDynCastAllOfMatcher<
   Stmt,
   StringLiteral> stringLiteral;
 
 /// \brief Matches character literals (also matches wchar_t).
 ///
 /// Not matching Hex-encoded chars (e.g. 0x1234, which is a IntegerLiteral),
 /// though.
 ///
 /// Example matches 'a', L'a'
 /// \code
-///   char ch = 'a'; wchar_t chw = L'a';
+///   char ch = 'a';
+///   wchar_t chw = L'a';
 /// \endcode
 const internal::VariadicDynCastAllOfMatcher<
   Stmt,
   CharacterLiteral> characterLiteral;
 
 /// \brief Matches integer literals of all sizes / encodings, e.g.
 /// 1, 1L, 0x1 and 1U.
 ///
 /// Does not match character-encoded integers such as L'a'.

 const internal::VariadicDynCastAllOfMatcher<
   Stmt,
   UserDefinedLiteral> userDefinedLiteral;
 
 /// \brief Matches compound (i.e. non-scalar) literals
 ///
 /// Example match: {1}, (1, 2)
 /// \code
-///   int array[4] = {1}; vector int myvec = (vector int)(1, 2);
+///   int array[4] = {1};
+///   vector int myvec = (vector int)(1, 2);
 /// \endcode
 const internal::VariadicDynCastAllOfMatcher<
   Stmt,
   CompoundLiteralExpr> compoundLiteralExpr;
 
 /// \brief Matches nullptr literal.
 const internal::VariadicDynCastAllOfMatcher<
   Stmt,

 ///     int b[] = { 2, 3 };
 ///     int c[b[0]];
 ///   }
 /// \endcode
 /// constantArrayType()
 ///   matches "int a[2]"
 AST_TYPE_MATCHER(ConstantArrayType, constantArrayType);
 
-/// \brief Matches \c ConstantArrayType nodes that have the specified size.
+/// \brief Matches nodes that have the specified size.
 ///
 /// Given
 /// \code
 ///   int a[42];
 ///   int b[2 * 21];
 ///   int c[41], d[43];
+///   char *s = "abcd";
+///   wchar_t *ws = L"abcd";
+///   char *w = "a";
 /// \endcode
 /// constantArrayType(hasSize(42))
 ///   matches "int a[42]" and "int b[2 * 21]"
-AST_MATCHER_P(ConstantArrayType, hasSize, unsigned, N) {
-  return Node.getSize() == N;
+/// stringLiteral(hasSize(4))
+///   matches "abcd", L"abcd"
+AST_POLYMORPHIC_MATCHER_P(hasSize,
+                          AST_POLYMORPHIC_SUPPORTED_TYPES(ConstantArrayType,
+                                                          StringLiteral),
+                          unsigned, N) {
+  return internal::HasSizeMatcher<NodeType>::hasSize(Node, N);
 }
 
 /// \brief Matches C++ arrays whose size is a value-dependent expression.
 ///
 /// Given
 /// \code
 ///   template<typename T, int Size>
 ///   class array {

include/clang/ASTMatchers/ASTMatchersInternal.h

 };
 
 template <>
 inline const Stmt *GetBodyMatcher<FunctionDecl>::get(const FunctionDecl &Node) {
   return Node.doesThisDeclarationHaveABody() ? Node.getBody() : nullptr;
 }
 
 template <typename Ty>
+struct HasSizeMatcher {
+  static bool hasSize(const Ty &Node, unsigned int N) {
+    return Node.getSize() == N;
+  }
+};
+
+template <>
+inline bool HasSizeMatcher<StringLiteral>::hasSize(
+    const StringLiteral &Node, unsigned int N) {
+  return Node.getLength() == N;
+}
+
+template <typename Ty>
 struct GetSourceExpressionMatcher {
   static const Expr *get(const Ty &Node) {
     return Node.getSubExpr();
   }
 };
 
 template <>
 inline const Expr *GetSourceExpressionMatcher<OpaqueValueExpr>::get(

unittests/ASTMatchers/ASTMatchersTest.cpp

TEST(Finder, DynamicOnlyAcceptsSomeMatchers) {
   MatchFinder Finder;
   EXPECT_TRUE(Finder.addDynamicMatcher(decl(), nullptr));
   EXPECT_TRUE(Finder.addDynamicMatcher(callExpr(), nullptr));
   EXPECT_TRUE(Finder.addDynamicMatcher(constantArrayType(hasSize(42)),
                                        nullptr));
 
   // Do not accept non-toplevel matchers.
   EXPECT_FALSE(Finder.addDynamicMatcher(isArrow(), nullptr));
-  EXPECT_FALSE(Finder.addDynamicMatcher(hasSize(2), nullptr));
   EXPECT_FALSE(Finder.addDynamicMatcher(hasName("x"), nullptr));
 }
 
 TEST(Decl, MatchesDeclarations) {
   EXPECT_TRUE(notMatches("", decl(usingDecl())));
   EXPECT_TRUE(matches("namespace x { class X {}; } using x::X;",
                       decl(usingDecl())));
 }
TEST(Matcher, StringLiterals) {
   // wide string
   EXPECT_TRUE(matches("const wchar_t *s = L\"string\";", Literal));
   // with escaped characters
   EXPECT_TRUE(matches("const char *s = \"\x05five\";", Literal));
   // no matching -- though the data type is the same, there is no string literal
   EXPECT_TRUE(notMatches("const char s[1] = {'a'};", Literal));
 }
 
+TEST(StringLiteral, HasSize) {
+  StatementMatcher Literal = stringLiteral(hasSize(4));
+  EXPECT_TRUE(matches("const char *s = \"abcd\";", Literal));
+  // wide string
+  EXPECT_TRUE(matches("const wchar_t *s = L\"abcd\";", Literal));
+  // with escaped characters
+  EXPECT_TRUE(matches("const char *s = \"\x05\x06\x07\x08\";", Literal));
+  // no matching, too small
+  EXPECT_TRUE(notMatches("const char *s = \"ab\";", Literal));
+}
+
 TEST(Matcher, CharacterLiterals) {
   StatementMatcher CharLiteral = characterLiteral();
   EXPECT_TRUE(matches("const char c = 'c';", CharLiteral));
   // wide character
   EXPECT_TRUE(matches("const char c = L'c';", CharLiteral));
   // wide character, Hex encoded, NOT MATCHED!
   EXPECT_TRUE(notMatches("const wchar_t c = 0x2126;", CharLiteral));
   EXPECT_TRUE(notMatches("const char c = 0x1;", CharLiteral));