Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(12)

Unified Diff: tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp

Issue 2589923002: Add a hardcoded list of methods that need a "Get" prefix. (Closed)
Patch Set: Created 4 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | tools/clang/rewrite_to_chrome_style/tests/methods-expected.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp
diff --git a/tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp b/tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp
index 97d9ebd309706e2e64d64d88f1452ea64400d3ed..176229efbbea873203f28e02452f264b91f3ca8f 100644
--- a/tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp
+++ b/tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp
@@ -340,6 +340,25 @@ AST_MATCHER_P(clang::QualType, hasString, std::string, ExpectedString) {
return ExpectedString == Node.getAsString();
}
+bool IsMethodNameLikelyToConflictWithTypeName(std::string method_name) {
dcheng 2016/12/19 19:26:52 Just call this ShouldPrefixMethodName ('likely' is
Łukasz Anforowicz 2016/12/19 21:58:47 Done.
+ // Make sure that IsMethodNameLikelyToConflictWithTypeName works
+ // both when |method_name| is the old and/or the new name.
+ method_name[0] = clang::toLowercase(method_name[0]);
dcheng 2016/12/19 19:26:52 As discussed, I think we shouldn't need this: - if
Łukasz Anforowicz 2016/12/19 21:58:47 Done. (OTOH, please note that this is why I also
+
+ // Methods that are named similarily to a type - they should be prefixed
+ // with a "get" prefix.
+ static const char* kConflictingMethods[] = {
+ "entryType",
+ "readyState",
Łukasz Anforowicz 2016/12/19 19:07:35 Nasko, can you help populate this list with more i
Łukasz Anforowicz 2016/12/20 00:32:05 I've populated this list with the errors I've seen
+ };
+ for (const auto& conflicting_method : kConflictingMethods) {
+ if (method_name == conflicting_method)
+ return true;
+ }
+
+ return false;
+}
+
bool GetNameForDecl(const clang::FunctionDecl& decl,
clang::ASTContext& context,
std::string& name) {
@@ -373,8 +392,13 @@ bool GetNameForDecl(const clang::FunctionDecl& decl,
// return type.
auto conflict_matcher =
functionDecl(returns(type_containing_same_name_as_function));
- if (IsMatching(conflict_matcher, decl, context))
+ bool return_type_conflicts_with_method_name =
+ IsMatching(conflict_matcher, decl, context);
+
+ if (return_type_conflicts_with_method_name ||
+ IsMethodNameLikelyToConflictWithTypeName(name)) {
name = "Get" + name;
+ }
return true;
}
@@ -829,6 +853,8 @@ class UnresolvedRewriterBase : public RewriterBase<TargetNode> {
!IsBlacklistedFunctionOrMethodName(old_name)) {
new_name = old_name;
new_name[0] = clang::toUppercase(old_name[0]);
+ if (IsMethodNameLikelyToConflictWithTypeName(new_name))
+ new_name = "Get" + new_name;
return true;
}
« no previous file with comments | « no previous file | tools/clang/rewrite_to_chrome_style/tests/methods-expected.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698