iOS linting at Pinterest
Michael Schneider | Pinterest engineer, iOS Core Experience
Coding consistency and guidelines are critical in a code base where a large group of developers work every day and new engineers join every week. In order to control code quality and enforce best practices, we recently added linting to our infrastructure to help developers write better code. Linting is the process of running a program that analyzes code for potential errors.
At the same time, we were moving our current build infrastructure to Bazel, so it was a good time to consider which linting options would work well with the new build. A main goal of this initiative was to push the linting process as early as possible to give developers an immediate response at compile time if code doesn’t align with our guidelines or best practices. In this post we’ll cover how we implemented linting and best practices learned along the way.
Clang Tools
We evaluated existing solutions, but none were flexible enough or would easily integrate into our build system. We then looked into what LLVM could provide, especially the tooling around Clang.
Clang provides infrastructure to write tools that need syntactic and semantic information about a program called Clang Tools. There are different interfaces Clang provides for developers to hook into the Clang compiling process. The available interfaces are LibClang, Clang Plugins and LibTooling. The page “Choosing the Right Interface for your Application” was especially helpful in deciding which Clang Tool interfaces would be best for our needs. We tested each interface on a small example and found LibClang wouldn’t have provided full control over the AST and LibTooling wouldn’t be able to run part of the build triggered by dependency changes. Ultimately we decided on the Clang Plugin.
An important thing to note is since Clang is written in C++, the tooling extensibility points themselves are exposed as C++ interfaces. Furthermore all documentation exists mostly as auto-generated doxygen output from the annotated C++ source code. So it’s very helpful to have previous C++ experience when working with Clang.
Clang Plugins
Clang Plugins enables us to run additional actions on the Clang AST (Abstract Syntax Tree) as part of a compilation. Clang’s AST is different from ASTs produced by other compilers in that it closely resembles both the written C++ code and the C++ standard. For example, parenthesis expressions and compile time constants are available in an unreduced form in the AST. This makes Clang’s AST a good fit for refactoring tools. (More info in Introduction to the Clang AST.)
Plugins are dynamic libraries that are loaded at runtime by the compiler and allow us to emit special lint-style warnings and errors to the compiler which will show up as the well-known warnings or errors in the Warning tab section on the left side of Xcode or directly within the source code. We call our Clang Plugin PINLinter.
Architecture
PINLinter, like every Clang plugin, is represented by a single dynamic library (dylib) that Clang loads at compile time. As soon as the PINLinter plugin is loaded, Clang uses a configuration file in JSON format containing a list of all rules that should be loaded, along with a unique name, and loads each listed linter rule. This allows us to have only one Clang Plugin for all rules instead of one for every rule, and we can enable or disable certain rules for different rule configurations like development vs. build machines.
PINLinterASTAction
The main Clang Plugin class’s name is PINLinterASTAction, and it’s a frontend action. A FrontendAction is an interface that allows execution of user-specific actions as part of the compilation. To run tools over the AST, Clang provides the convenience interface PluginASTAction which takes care of executing the action by implementing the CreateASTConsumer method that returns an ASTConsumer per translation unit.
A plugin is loaded from a dynamic library at runtime by the compiler. To register the PINLinter plugin in the library, we use the FrontendPluginRegistry::Add<> method:
static clang::FrontendPluginRegistry::Add<PINLinterASTAction> X("PINLinter", "LLVM Pinterest Linter Plugin");}
In PINLinter’s implementation of CreateConsumer, a configuration file will be parsed for the list of rules that should be loaded. The configuration file is very simple and looks like the following.
{ // ... linters: [ "PINPreventAssignPointersRule", "PINCallViewInInitRule", // ... ] // ...
}
For every rule that’s listed within the configuration file, a concrete object will be created via CreateRulesForConfiguration. The vector of rule objects are then passed to the initializer of the PINLinterASTConsumer class.
// Central place where the linters plugin is registeredclass PINLinterASTAction : public PluginASTAction {// ...protected: virtual std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI, StringRef InFile) { // Try read linters path from environment variables Optional<std::string> env = sys::Process::GetEnv("PINLINTERS_CONFIGURATION"); if (env.hasValue()) { CreateRulesForConfiguration(env.getValue()); } return make_unique<PINLinterASTConsumer>(std::move(m_rules)); } // ...
}
PINLinterASTConsumer
An ASTConsumer is a client object that receives callbacks from Clang as the AST is built and “consumes” it. The PINLinterASTConsumer itself is kind of the glue between the PINLinterASTAction and all PINLinter::Rule objects. Its main job is to forward calls of the overwritten Initialize and HandleTranslationUnit methods to all loaded rules it’s aware of.
class PINLinterASTConsumer : public ASTConsumer { // ...
virtual void Initialize(ASTContext &Context) override { // Initialize all loaded linters for (auto &linter : m_linter) { linter->Initialize(Context); } }
virtual void HandleTranslationUnit(ASTContext &context) override { // Pass through call to all registered linter for (auto &linter : m_linter) { linter->HandleTranslationUnit(context); } } // ...};
PINLinter::Rule
Each linter rule is a subclass of the PINLinter::Rule class. PINLinter::Rule subclasses have two main purposes. The first is to initialize itself by overwriting Initialize to create a top level RecursiveASTVisitor and dispatch the high level translation unit in HandleTranslationUnit to the aforementioned root AST visitor object. An example implementation for a concrete rule subclass looks like the following:
class PINPreventAssignPointersRule : public PINLinter::Rule { // ... virtual void Initialize(ASTContext &Context) override { PINLinter::Rule::Initialize(Context); _Visitor = make_unique<PINPreventAssignPointersLinterVisitor>(Context); }
virtual void HandleTranslationUnit(ASTContext &Context) override { PINLinter::Rule::HandleTranslationUnit(Context); _Visitor->TraverseDecl(Context.getTranslationUnitDecl()); } // ...
};
As mentioned, the PINLinterASTAction is responsible for creating and initializing concrete rule objects based on a local configuration file. The ability to look up the available linter rule classes provides one central registry object called PINLinter::registry::RuleRegistry where every rule class can register itself with a unique name. To make this registration process easier, we created a macro called RULE_REGISTER for easy registration. For example, here we register a rule class called PINPreventAssignPointersRule for usage:
RULE_REGISTER(PINPreventAssignPointersRule, "PINPreventAssignPointersRule")RecursiveASTVisitor
RecursiveASTVisitor objects, which PINLinter::Rule classes create within their Initialize method, complete preorder or postorder depth-first traversal on the entire Clang AST and visit each node. In our case, it starts the process by calling TraverseDecl with the root level translation unit declaration. By default, this visitor pre-order traverses the AST. If post-order traversal is needed, the shouldTraversePostOrder method needs to be overridden to return true.
The RecursiveASTVisitor class performs three distinct tasks:
- Traverses the AST (i.e. go to each node).
- At a given node, walk up the class hierarchy starting from the node’s dynamic type until the top-most class (e.g. Stmt, Decl or Type) is reached.
- Given a (node, class) combination, where ‘class’ is some base class of the dynamic type of ‘node,’ call a user-overridable function to actually visit the node.
These tasks are done by three groups of methods, respectively:
TraverseDecl(Decl *x)does task #1. It’s the entry point for traversing an AST rooted at x. This method simply dispatches (i.e. forwards) toTraverseFoo(Foo *x)whereFoois the dynamic type of*x, which callsWalkUpFromFoo(x)and then recursively visits the child nodes ofx.TraverseStmt(Stmt *x)andTraverseType(QualType x)works similarly.WalkUpFromFoo(Foo *x)does task #2. It doesn’t try to visit any child node ofx. Instead, it first callsWalkUpFromBar(x)where Bar is the direct parent class ofFoo(unlessFoohas no parent), and then callsVisitFoo(x)(see the next list item).VisitFoo(Foo *x)does task #3.
For our purposes, we’re only interested in the third task from above. To hook into the parsing process, we have to subclass the RecursiveASTVisitor class (providing ourselves as the template argument, using the template pattern) and override any of the Visit* methods for declarations, types, statements, expressions or other AST nodes where the visitor should customize behavior.
For example if we wanted to check certain cases against Objective-C method declarations we have to implement bool VisitObjCMethodDecl(ObjCMethodDecl *D) and use the ObjCMethodDecl object that’s passed in as argument to do further checks and emit warnings or errors based on findings.
Example of implemented rules
To show how concrete PINLinter::Rule implementations look like in practice, let’s look into two examples we’re currently running in our code base.
Prevent assign property for Objective-C pointer
One linter rule we implemented prevents an assign property for an Objective-C type. Against the following scenarios, it will be checked as soon as the developer hits compile in Xcode:
// Error using pointer for a scalar type@property (nonatomic, assign) CGFloat *foo;// Error using assign for an objc pointer type@property (nonatomic, assign) NSNumber *bar;
To check against a specific Objective-C property declaration, we’re overwriting the VisitObjCPropertyDecl method in our RecursiveASTVisitor subclass. This method gets called for each Objective-C property declaration. The concrete implementation looks similar to the following:
class PINPreventAssignPointersLinterVisitor : public RecursiveASTVisitor<PINPreventAssignPointersLinterVisitor> { // … bool VisitObjCPropertyDecl(ObjCPropertyDecl *P) { // … // Check for pointer type QualType Type = P->getType(); // We are interested in any pointer type, but for now don’t bother for void // pointers as well as pointer to pointers if (Type->isAnyPointerType() && !Type->isVoidPointerType() && !isPointerToPointerType(Type)) { // Check if the written properties contains the one we are interested const auto PropertyAttributes = P->getPropertyAttributes(); const auto IsAssignPointer = (PropertyAttributes & (ObjCPropertyDecl::PropertyAttributeKind::OBJC_PR_assign | ObjCPropertyDecl::PropertyAttributeKind:: OBJC_PR_unsafe_unretained)); if (IsAssignPointer) { std::string WarningString = ([Type]() { if (Type->isObjCObjectPointerType()) { return “Using assign for an ObjC pointer type is not valid. Either “ “use strong or weak for a zeroing reference in this case.”; } else { return “Using a pointer for a scalar type is invalid.”; } }()); // Emit warning / error auto &DiagnosticEngine = _Context.getDiagnostics(); unsigned DiagnosticID = DiagnosticEngine.getCustomDiagID(DiagnosticsEngine::Warning, “%0”); DiagnosticEngine.Report(P->getLocStart(), DiagnosticID) << WarningString; } } // … } //…
};
As soon as the developer tries to compile the first time after introducing new code, Xcode will immediately show a warning with a tip explaining how to fix the issue:
Texture: Accessing view property in init method
Pinterest’s iOS app is almost exclusively written in Texture (formerly AsyncDisplayKit). There are common patterns that can reduce Texture’s ability to increase performance, and since we own the linter infrastructure, we create rules specifically target these anti-patterns.
For instance, one of these patterns is accessing the view property of an ASDisplayNode in any kind of init method. Accessing the view of the ASDisplayNode will load its backing view or layer. However, the init method could be called any thread, so it’s not thread safe to do this, because the loading process can cause accessing properties of the backing UIView or CALayer objects.
A custom built linting infrastructure allows us the flexibility to make changes to new requirements quickly without relying on other linter infrastructure to add rules for our needs in their portfolio.
Escape hatch
In certain circumstances it’s necessary to skip linting. PINLinter provides an escape hatch. We have full access to the AST and can look for custom-defined properties, skipping the linter rule completely by adding a custom attribution that can be attached to functions, methods or properties:
// functions__attribute__((annotate(“pi_nolint”)))static int divide(int denominator, int numerator) { /* … */ }
// methods- (ASCellNodeBlock)tableNode:(ASTableNode *)tableNode nodeBlockForRowAtIndexPath:(NSIndexPath *)indexPath __attribute__((annotate(“pi_nolint”))) { /*… */ }
// properties__attribute__((annotate(“pi_nolint”)))@property (assign) CGFloat *iKnowWhatImDoint;
If this attribute is attached to a declaration, the linter rule will skip it and no warning will be shown. This option is only used in special cases. We added a specific rule in our code review infrastructure that automatically adds a specific group of people to the Differential for review if the pi_nolint attribution is used.
Caveats
Custom Clang Version
To use Clang it’s necessary to build Clang from source or use a pre-build version from the LLVM website. The Clang version that’s shipping via Xcode doesn’t support Clang plugins out of the box, so we download a pre-build Clang binary during setup for every developer and point Xcode to use this version of Clang for the compiling process. This is possible by setting the CC and the CXX build variable to the path where the custom Clang binary lives. Every next compile process will then use this Clang binary instead of the Xcode one.
Unknown attribute noescape
Another issue we ran into was the pre-compiled Clang binary didn’t understand the no escape attribute, and we saw compile errors the first time we compiled Pinterest’s iOS app.
To resolve this issue we created a macro that expands if the compiler supports no_escape and replaced all of the __attribute__((noescape)) appearances with it.
#if __has_attribute(noescape) #define PI_NOESCAPE __attribute__((noescape))#else #define PI_NOESCAPE#endif
Summary
Overall we’re seeing major gains with how this infrastructure works together as well as significant improvements in code quality and developer happiness.
Acknowledgements: Thank you to all our iOS developers for using and giving feedback, especially my teammates Garrett Moon, Jon Parise, Brandon Kase, Rahul Malik and Levi McCallum for feedback on this post.
