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

Unified Diff: tools/clang/plugins/ChromeClassTester.cpp

Issue 2780113002: Refactor banned directory checking so Blink can opt into certain checks. (Closed)
Patch Set: better enum names Created 3 years, 8 months 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 | « tools/clang/plugins/ChromeClassTester.h ('k') | tools/clang/plugins/FindBadConstructsConsumer.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: tools/clang/plugins/ChromeClassTester.cpp
diff --git a/tools/clang/plugins/ChromeClassTester.cpp b/tools/clang/plugins/ChromeClassTester.cpp
index b0cc3db5aed56da1435b1ec19dff3c48fa3ff423..c9388067234f44a1985587dd660edfd1417b1e1c 100644
--- a/tools/clang/plugins/ChromeClassTester.cpp
+++ b/tools/clang/plugins/ChromeClassTester.cpp
@@ -48,15 +48,15 @@ void ChromeClassTester::CheckTag(TagDecl* tag) {
// We handle class types here where we have semantic information. We can only
// check structs/classes/enums here, but we get a bunch of nice semantic
// information instead of just parsing information.
+ if (InBannedNamespace(tag))
+ return;
- if (CXXRecordDecl* record = dyn_cast<CXXRecordDecl>(tag)) {
- if (InBannedNamespace(record))
- return;
-
- SourceLocation record_location = record->getInnerLocStart();
- if (InBannedDirectory(record_location))
- return;
+ SourceLocation location = tag->getInnerLocStart();
+ LocationType location_type = ClassifyLocation(location);
+ if (location_type == LocationType::kThirdParty)
+ return;
+ if (CXXRecordDecl* record = dyn_cast<CXXRecordDecl>(tag)) {
// We sadly need to maintain a blacklist of types that violate these
// rules, but do so for good reason or due to limitations of this
// checker (i.e., we don't handle extern templates very well).
@@ -69,17 +69,14 @@ void ChromeClassTester::CheckTag(TagDecl* tag) {
if (ends_with(base_name, "Matcher"))
return;
- CheckChromeClass(record_location, record);
+ CheckChromeClass(location_type, location, record);
} else if (EnumDecl* enum_decl = dyn_cast<EnumDecl>(tag)) {
- SourceLocation enum_location = enum_decl->getInnerLocStart();
- if (InBannedDirectory(enum_location))
- return;
-
std::string base_name = enum_decl->getNameAsString();
+ // TODO(dcheng): This should probably consult a separate list.
if (IsIgnoredType(base_name))
return;
- CheckChromeEnum(enum_location, enum_decl);
+ CheckChromeEnum(location_type, location, enum_decl);
}
}
@@ -98,27 +95,27 @@ void ChromeClassTester::emitWarning(SourceLocation loc,
}
-bool ChromeClassTester::InBannedDirectory(SourceLocation loc) {
+ChromeClassTester::LocationType ChromeClassTester::ClassifyLocation(
+ SourceLocation loc) {
if (instance().getSourceManager().isInSystemHeader(loc))
- return true;
+ return LocationType::kThirdParty;
std::string filename;
if (!GetFilename(loc, &filename)) {
// If the filename cannot be determined, simply treat this as a banned
// location, instead of going through the full lookup process.
- return true;
+ return LocationType::kThirdParty;
}
// We need to special case scratch space; which is where clang does its
// macro expansion. We explicitly want to allow people to do otherwise bad
// things through macros that were defined due to third party libraries.
if (filename == "<scratch space>")
- return true;
+ return LocationType::kThirdParty;
// Don't complain about autogenerated protobuf files.
- if (ends_with(filename, ".pb.h")) {
- return true;
- }
+ if (ends_with(filename, ".pb.h"))
+ return LocationType::kThirdParty;
#if defined(LLVM_ON_UNIX)
// Resolve the symlinktastic relative path and make it absolute.
@@ -160,14 +157,14 @@ bool ChromeClassTester::InBannedDirectory(SourceLocation loc) {
std::replace(filename.begin(), filename.end(), '\\', '/');
#endif
- for (const std::string& allowed_dir : allowed_directories_) {
+ for (const std::string& blink_dir : blink_directories_) {
// If any of the allowed directories occur as a component in filename,
// this file is allowed.
- assert(allowed_dir.front() == '/' && "Allowed dir must start with '/'");
- assert(allowed_dir.back() == '/' && "Allowed dir must end with '/'");
+ assert(blink_dir.front() == '/' && "Allowed dir must start with '/'");
+ assert(blink_dir.back() == '/' && "Allowed dir must end with '/'");
- if (filename.find(allowed_dir) != std::string::npos)
- return false;
+ if (filename.find(blink_dir) != std::string::npos)
+ return LocationType::kBlink;
}
for (const std::string& banned_dir : banned_directories_) {
@@ -177,24 +174,22 @@ bool ChromeClassTester::InBannedDirectory(SourceLocation loc) {
assert(banned_dir.back() == '/' && "Banned dir must end with '/'");
if (filename.find(banned_dir) != std::string::npos)
- return true;
+ return LocationType::kThirdParty;
}
- return false;
+ return LocationType::kChrome;
}
bool ChromeClassTester::InBannedNamespace(const Decl* record) {
std::string n = GetNamespace(record);
- if (!n.empty()) {
- return std::find(banned_namespaces_.begin(), banned_namespaces_.end(), n)
- != banned_namespaces_.end();
- }
+ if (!n.empty())
+ return banned_namespaces_.find(n) != banned_namespaces_.end();
return false;
}
std::string ChromeClassTester::GetNamespace(const Decl* record) {
- return GetNamespaceImpl(record->getDeclContext(), "");
+ return GetNamespaceImpl(record->getDeclContext(), std::string());
}
bool ChromeClassTester::HasIgnoredBases(const CXXRecordDecl* record) {
@@ -238,9 +233,7 @@ void ChromeClassTester::BuildBannedLists() {
banned_namespaces_.emplace("std");
banned_namespaces_.emplace("__gnu_cxx");
- if (options_.enforce_in_thirdparty_webkit) {
- allowed_directories_.emplace("/third_party/WebKit/");
- }
+ blink_directories_.emplace("/third_party/WebKit/");
banned_directories_.emplace("/third_party/");
banned_directories_.emplace("/native_client/");
« no previous file with comments | « tools/clang/plugins/ChromeClassTester.h ('k') | tools/clang/plugins/FindBadConstructsConsumer.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698