Fix clang-tidy warnings in aapt and aapt2.

* Add explicit keyword to conversion constructors.
* Add NOLINT(implicit) comments for implicit conversion constructors.
Bug: 28341362
* Use const reference type for read-only parameters.
Bug: 30407689
* Add NOLINT to suppress wrong clang-tidy warnings on macros.
Test: build with WITH_TIDY=1

Change-Id: Ibfafb23d9be463b4072745a51a6d9f1812dcac45

22 files changed, 44 insertions, 43 deletions

diff --git a/tools/aapt2/Diagnostics.h b/tools/aapt2/Diagnostics.h
index e86f2a8830e8..725027c2c45a 100644
--- a/tools/aapt2/Diagnostics.h
+++ b/tools/aapt2/Diagnostics.h
@@ -41,13 +41,13 @@ private:
 public:
     DiagMessage() = default;
 
-    DiagMessage(const StringPiece& src) : mSource(src) {
+    explicit DiagMessage(const StringPiece& src) : mSource(src) {
     }
 
-    DiagMessage(const Source& src) : mSource(src) {
+    explicit DiagMessage(const Source& src) : mSource(src) {
     }
 
-    DiagMessage(size_t line) : mSource(Source().withLine(line)) {
+    explicit DiagMessage(size_t line) : mSource(Source().withLine(line)) {
     }
 
     template <typename T>
diff --git a/tools/aapt2/NameMangler.h b/tools/aapt2/NameMangler.h
index 505a982e5eab..b6aaa4df3bec 100644
--- a/tools/aapt2/NameMangler.h
+++ b/tools/aapt2/NameMangler.h
@@ -43,7 +43,7 @@ private:
     NameManglerPolicy mPolicy;
 
 public:
-    NameMangler(NameManglerPolicy policy) : mPolicy(policy) {
+    explicit NameMangler(NameManglerPolicy policy) : mPolicy(policy) {
     }
 
     Maybe<ResourceName> mangleName(const ResourceName& name) {
diff --git a/tools/aapt2/Source.h b/tools/aapt2/Source.h
index 319528e0ea1b..8a1021d49c39 100644
--- a/tools/aapt2/Source.h
+++ b/tools/aapt2/Source.h
@@ -35,7 +35,7 @@ struct Source {
 
     Source() = default;
 
-    inline Source(const StringPiece& path) : path(path.toString()) {
+    inline Source(const StringPiece& path) : path(path.toString()) {  // NOLINT(implicit)
     }
 
     inline Source(const StringPiece& path, size_t line) : path(path.toString()), line(line) {
diff --git a/tools/aapt2/ValueVisitor.h b/tools/aapt2/ValueVisitor.h
index b8bc5db7d6e4..9dc6a9c2b0d2 100644
--- a/tools/aapt2/ValueVisitor.h
+++ b/tools/aapt2/ValueVisitor.h
@@ -45,8 +45,9 @@ struct RawValueVisitor {
     virtual void visit(Styleable* value) {}
 };
 
+// NOLINT, do not add parentheses around T.
 #define DECL_VISIT_COMPOUND_VALUE(T) \
-    virtual void visit(T* value) { \
+    virtual void visit(T* value) { /* NOLINT */ \
         visitSubValues(value); \
     }
 
diff --git a/tools/aapt2/compile/Png.h b/tools/aapt2/compile/Png.h
index 345ff6c56870..f835b06e762b 100644
--- a/tools/aapt2/compile/Png.h
+++ b/tools/aapt2/compile/Png.h
@@ -32,7 +32,7 @@ struct PngOptions {
 
 class Png {
 public:
-    Png(IDiagnostics* diag) : mDiag(diag) {
+    explicit Png(IDiagnostics* diag) : mDiag(diag) {
     }
 
     bool process(const Source& source, std::istream* input, BigBuffer* outBuffer,
diff --git a/tools/aapt2/compile/Pseudolocalizer.h b/tools/aapt2/compile/Pseudolocalizer.h
index 7db88de13536..91d17d174d29 100644
--- a/tools/aapt2/compile/Pseudolocalizer.h
+++ b/tools/aapt2/compile/Pseudolocalizer.h
@@ -43,7 +43,7 @@ public:
         kBidi,
     };
 
-    Pseudolocalizer(Method method);
+    explicit Pseudolocalizer(Method method);
     void setMethod(Method method);
     std::string start() { return mImpl->start(); }
     std::string end() { return mImpl->end(); }
diff --git a/tools/aapt2/flatten/TableFlattener.h b/tools/aapt2/flatten/TableFlattener.h
index 0ab01974044b..b416f202ced5 100644
--- a/tools/aapt2/flatten/TableFlattener.h
+++ b/tools/aapt2/flatten/TableFlattener.h
@@ -26,7 +26,7 @@ class ResourceTable;
 
 class TableFlattener : public IResourceTableConsumer {
 public:
-    TableFlattener(BigBuffer* buffer) : mBuffer(buffer) {
+    explicit TableFlattener(BigBuffer* buffer) : mBuffer(buffer) {
     }
 
     bool consume(IAaptContext* context, ResourceTable* table) override;
diff --git a/tools/aapt2/io/FileSystem.h b/tools/aapt2/io/FileSystem.h
index f0559c03a8b8..72a932a499dd 100644
--- a/tools/aapt2/io/FileSystem.h
+++ b/tools/aapt2/io/FileSystem.h
@@ -29,7 +29,7 @@ namespace io {
  */
 class RegularFile : public IFile {
 public:
-    RegularFile(const Source& source);
+    explicit RegularFile(const Source& source);
 
     std::unique_ptr<IData> openAsData() override;
     const Source& getSource() const override;
@@ -42,7 +42,7 @@ class FileCollection;
 
 class FileCollectionIterator : public IFileCollectionIterator {
 public:
-    FileCollectionIterator(FileCollection* collection);
+    explicit FileCollectionIterator(FileCollection* collection);
 
     bool hasNext() override;
     io::IFile* next() override;
diff --git a/tools/aapt2/io/ZipArchive.h b/tools/aapt2/io/ZipArchive.h
index 5ad119d1d6d4..565588e2db57 100644
--- a/tools/aapt2/io/ZipArchive.h
+++ b/tools/aapt2/io/ZipArchive.h
@@ -47,7 +47,7 @@ class ZipFileCollection;
 
 class ZipFileCollectionIterator : public IFileCollectionIterator {
 public:
-    ZipFileCollectionIterator(ZipFileCollection* collection);
+    explicit ZipFileCollectionIterator(ZipFileCollection* collection);
 
     bool hasNext() override;
     io::IFile* next() override;
diff --git a/tools/aapt2/java/ClassDefinition.h b/tools/aapt2/java/ClassDefinition.h
index d45328fedba2..e84c2741e4ce 100644
--- a/tools/aapt2/java/ClassDefinition.h
+++ b/tools/aapt2/java/ClassDefinition.h
@@ -110,7 +110,7 @@ using StringMember = PrimitiveMember<std::string>;
 template <typename T>
 class PrimitiveArrayMember : public ClassMember {
 public:
-    PrimitiveArrayMember(const StringPiece& name) :
+    explicit PrimitiveArrayMember(const StringPiece& name) :
             mName(name.toString()) {
     }
 
diff --git a/tools/aapt2/link/ManifestFixer.h b/tools/aapt2/link/ManifestFixer.h
index 55b587e57013..2e81266015e1 100644
--- a/tools/aapt2/link/ManifestFixer.h
+++ b/tools/aapt2/link/ManifestFixer.h
@@ -41,7 +41,7 @@ struct ManifestFixerOptions {
  */
 class ManifestFixer : public IXmlResourceConsumer {
 public:
-    ManifestFixer(const ManifestFixerOptions& options) : mOptions(options) {
+    explicit ManifestFixer(const ManifestFixerOptions& options) : mOptions(options) {
     }
 
     bool consume(IAaptContext* context, xml::XmlResource* doc) override;
diff --git a/tools/aapt2/link/ProductFilter.h b/tools/aapt2/link/ProductFilter.h
index d2edd38289dc..7724e140427b 100644
--- a/tools/aapt2/link/ProductFilter.h
+++ b/tools/aapt2/link/ProductFilter.h
@@ -29,7 +29,7 @@ class ProductFilter {
 public:
     using ResourceConfigValueIter = std::vector<std::unique_ptr<ResourceConfigValue>>::iterator;
 
-    ProductFilter(std::unordered_set<std::string> products) : mProducts(products) { }
+    explicit ProductFilter(std::unordered_set<std::string> products) : mProducts(products) { }
 
     ResourceConfigValueIter selectProductToKeep(const ResourceNameRef& name,
                                                 const ResourceConfigValueIter begin,
diff --git a/tools/aapt2/process/SymbolTable.h b/tools/aapt2/process/SymbolTable.h
index bd01b64281d5..bd31416a5cee 100644
--- a/tools/aapt2/process/SymbolTable.h
+++ b/tools/aapt2/process/SymbolTable.h
@@ -54,7 +54,7 @@ public:
         Symbol() : Symbol(Maybe<ResourceId>{}) {
         }
 
-        Symbol(const Maybe<ResourceId>& i) : Symbol(i, nullptr) {
+        explicit Symbol(const Maybe<ResourceId>& i) : Symbol(i, nullptr) {
         }
 
         Symbol(const Maybe<ResourceId>& i, const std::shared_ptr<Attribute>& attr) :
diff --git a/tools/aapt2/test/Builders.h b/tools/aapt2/test/Builders.h
index c0c016032921..637e991a573f 100644
--- a/tools/aapt2/test/Builders.h
+++ b/tools/aapt2/test/Builders.h
@@ -46,12 +46,12 @@ public:
         return *this;
     }
 
-    ResourceTableBuilder& addSimple(const StringPiece& name, const ResourceId id = {}) {
+    ResourceTableBuilder& addSimple(const StringPiece& name, const ResourceId& id = {}) {
         return addValue(name, id, util::make_unique<Id>());
     }
 
     ResourceTableBuilder& addSimple(const StringPiece& name, const ConfigDescription& config,
-                                    const ResourceId id = {}) {
+                                    const ResourceId& id = {}) {
         return addValue(name, config, id, util::make_unique<Id>());
     }
 
@@ -59,7 +59,7 @@ public:
         return addReference(name, {}, ref);
     }
 
-    ResourceTableBuilder& addReference(const StringPiece& name, const ResourceId id,
+    ResourceTableBuilder& addReference(const StringPiece& name, const ResourceId& id,
                                        const StringPiece& ref) {
         return addValue(name, id, util::make_unique<Reference>(parseNameOrDie(ref)));
     }
@@ -68,12 +68,12 @@ public:
         return addString(name, {}, str);
     }
 
-    ResourceTableBuilder& addString(const StringPiece& name, const ResourceId id,
+    ResourceTableBuilder& addString(const StringPiece& name, const ResourceId& id,
                                     const StringPiece& str) {
         return addValue(name, id, util::make_unique<String>(mTable->stringPool.makeRef(str)));
     }
 
-    ResourceTableBuilder& addString(const StringPiece& name, const ResourceId id,
+    ResourceTableBuilder& addString(const StringPiece& name, const ResourceId& id,
                                     const ConfigDescription& config, const StringPiece& str) {
         return addValue(name, config, id,
                         util::make_unique<String>(mTable->stringPool.makeRef(str)));
@@ -83,7 +83,7 @@ public:
         return addFileReference(name, {}, path);
     }
 
-    ResourceTableBuilder& addFileReference(const StringPiece& name, const ResourceId id,
+    ResourceTableBuilder& addFileReference(const StringPiece& name, const ResourceId& id,
                                            const StringPiece& path) {
         return addValue(name, id,
                         util::make_unique<FileReference>(mTable->stringPool.makeRef(path)));
@@ -100,13 +100,13 @@ public:
         return addValue(name, {}, std::move(value));
     }
 
-    ResourceTableBuilder& addValue(const StringPiece& name, const ResourceId id,
+    ResourceTableBuilder& addValue(const StringPiece& name, const ResourceId& id,
                                    std::unique_ptr<Value> value) {
         return addValue(name, {}, id, std::move(value));
     }
 
     ResourceTableBuilder& addValue(const StringPiece& name, const ConfigDescription& config,
-                                   const ResourceId id, std::unique_ptr<Value> value) {
+                                   const ResourceId& id, std::unique_ptr<Value> value) {
         ResourceName resName = parseNameOrDie(name);
         bool result = mTable->addResourceAllowMangled(resName, id, config, {},
                                                       std::move(value), &mDiagnostics);
@@ -114,7 +114,7 @@ public:
         return *this;
     }
 
-    ResourceTableBuilder& setSymbolState(const StringPiece& name, ResourceId id,
+    ResourceTableBuilder& setSymbolState(const StringPiece& name, const ResourceId& id,
                                          SymbolState state) {
         ResourceName resName = parseNameOrDie(name);
         Symbol symbol;
@@ -130,7 +130,7 @@ public:
 };
 
 inline std::unique_ptr<Reference> buildReference(const StringPiece& ref,
-                                                 Maybe<ResourceId> id = {}) {
+                                                 const Maybe<ResourceId>& id = {}) {
     std::unique_ptr<Reference> reference = util::make_unique<Reference>(parseNameOrDie(ref));
     reference->id = id;
     return reference;
@@ -151,7 +151,7 @@ private:
 
 public:
     template <typename... Args>
-    ValueBuilder(Args&&... args) : mValue(new T{ std::forward<Args>(args)... }) {
+    explicit ValueBuilder(Args&&... args) : mValue(new T{ std::forward<Args>(args)... }) {
     }
 
     template <typename... Args>
@@ -175,7 +175,7 @@ private:
     std::unique_ptr<Attribute> mAttr;
 
 public:
-    AttributeBuilder(bool weak = false) : mAttr(util::make_unique<Attribute>(weak)) {
+    explicit AttributeBuilder(bool weak = false) : mAttr(util::make_unique<Attribute>(weak)) {
         mAttr->typeMask = android::ResTable_map::TYPE_ANY;
     }
 
@@ -211,7 +211,7 @@ public:
         return *this;
     }
 
-    StyleBuilder& addItem(const StringPiece& str, ResourceId id, std::unique_ptr<Item> value) {
+    StyleBuilder& addItem(const StringPiece& str, const ResourceId& id, std::unique_ptr<Item> value) {
         addItem(str, std::move(value));
         mStyle->entries.back().key.id = id;
         return *this;
@@ -227,7 +227,7 @@ private:
     std::unique_ptr<Styleable> mStyleable = util::make_unique<Styleable>();
 
 public:
-    StyleableBuilder& addItem(const StringPiece& str, Maybe<ResourceId> id = {}) {
+    StyleableBuilder& addItem(const StringPiece& str, const Maybe<ResourceId>& id = {}) {
         mStyleable->entries.push_back(Reference(parseNameOrDie(str)));
         mStyleable->entries.back().id = id;
         return *this;
diff --git a/tools/aapt2/test/Common.h b/tools/aapt2/test/Common.h
index 624a71a382c9..7fafcbeb1a04 100644
--- a/tools/aapt2/test/Common.h
+++ b/tools/aapt2/test/Common.h
@@ -104,7 +104,7 @@ private:
     Source mSource;
 
 public:
-    TestFile(const StringPiece& path) : mSource(path) {}
+    explicit TestFile(const StringPiece& path) : mSource(path) {}
 
     std::unique_ptr<io::IData> openAsData() override {
         return {};
diff --git a/tools/aapt2/test/Context.h b/tools/aapt2/test/Context.h
index b053e07eafde..54f16db3d2f9 100644
--- a/tools/aapt2/test/Context.h
+++ b/tools/aapt2/test/Context.h
@@ -87,7 +87,7 @@ public:
         return *this;
     }
 
-    ContextBuilder& setNameManglerPolicy(NameManglerPolicy policy) {
+    ContextBuilder& setNameManglerPolicy(const NameManglerPolicy& policy) {
         mContext->mNameMangler = NameMangler(policy);
         return *this;
     }
diff --git a/tools/aapt2/util/BigBuffer.h b/tools/aapt2/util/BigBuffer.h
index cad2a2e63be1..ba8532f829e6 100644
--- a/tools/aapt2/util/BigBuffer.h
+++ b/tools/aapt2/util/BigBuffer.h
@@ -63,7 +63,7 @@ public:
      * Create a BigBuffer with block allocation sizes
      * of blockSize.
      */
-    BigBuffer(size_t blockSize);
+    explicit BigBuffer(size_t blockSize);
 
     BigBuffer(const BigBuffer&) = delete; // No copying.
 
diff --git a/tools/aapt2/util/Files.h b/tools/aapt2/util/Files.h
index 4d8a1feb63b1..52c2052aec3d 100644
--- a/tools/aapt2/util/Files.h
+++ b/tools/aapt2/util/Files.h
@@ -109,7 +109,7 @@ bool appendArgsFromFile(const StringPiece& path, std::vector<std::string>* outAr
  */
 class FileFilter {
 public:
-    FileFilter(IDiagnostics* diag) : mDiag(diag) {
+    explicit FileFilter(IDiagnostics* diag) : mDiag(diag) {
     }
 
     /*
diff --git a/tools/aapt2/util/Maybe.h b/tools/aapt2/util/Maybe.h
index a31bc89fa98b..129f6d9d9716 100644
--- a/tools/aapt2/util/Maybe.h
+++ b/tools/aapt2/util/Maybe.h
@@ -43,12 +43,12 @@ public:
     Maybe(const Maybe& rhs);
 
     template <typename U>
-    Maybe(const Maybe<U>& rhs);
+    Maybe(const Maybe<U>& rhs);  // NOLINT(implicit)
 
     Maybe(Maybe&& rhs);
 
     template <typename U>
-    Maybe(Maybe<U>&& rhs);
+    Maybe(Maybe<U>&& rhs);  // NOLINT(implicit)
 
     Maybe& operator=(const Maybe& rhs);
 
@@ -63,12 +63,12 @@ public:
     /**
      * Construct a Maybe holding a value.
      */
-    Maybe(const T& value);
+    Maybe(const T& value);  // NOLINT(implicit)
 
     /**
      * Construct a Maybe holding a value.
      */
-    Maybe(T&& value);
+    Maybe(T&& value);  // NOLINT(implicit)
 
     /**
      * True if this holds a value, false if
diff --git a/tools/aapt2/util/StringPiece.h b/tools/aapt2/util/StringPiece.h
index f5c985bd9fff..4300a67d3581 100644
--- a/tools/aapt2/util/StringPiece.h
+++ b/tools/aapt2/util/StringPiece.h
@@ -41,8 +41,8 @@ public:
 
     BasicStringPiece();
     BasicStringPiece(const BasicStringPiece<TChar>& str);
-    BasicStringPiece(const std::basic_string<TChar>& str);
-    BasicStringPiece(const TChar* str);
+    BasicStringPiece(const std::basic_string<TChar>& str);  // NOLINT(implicit)
+    BasicStringPiece(const TChar* str);  // NOLINT(implicit)
     BasicStringPiece(const TChar* str, size_t len);
 
     BasicStringPiece<TChar>& operator=(const BasicStringPiece<TChar>& rhs);
diff --git a/tools/aapt2/util/Util.h b/tools/aapt2/util/Util.h
index 4a10987b4400..112437dc7bde 100644
--- a/tools/aapt2/util/Util.h
+++ b/tools/aapt2/util/Util.h
@@ -242,7 +242,7 @@ private:
     const iterator mEnd;
 };
 
-inline Tokenizer tokenize(StringPiece str, char sep) {
+inline Tokenizer tokenize(const StringPiece& str, char sep) {
     return Tokenizer(str, sep);
 }
 
@@ -281,7 +281,7 @@ bool extractResFilePathParts(const StringPiece& path, StringPiece* outPrefix,
  * In the aapt namespace for lookup.
  */
 inline ::std::ostream& operator<<(::std::ostream& out,
-                                  ::std::function<::std::ostream&(::std::ostream&)> f) {
+                                  const ::std::function<::std::ostream&(::std::ostream&)>& f) {
     return f(out);
 }
 
diff --git a/tools/aapt2/xml/XmlPullParser.h b/tools/aapt2/xml/XmlPullParser.h
index ee51b360feb4..a24d109abf34 100644
--- a/tools/aapt2/xml/XmlPullParser.h
+++ b/tools/aapt2/xml/XmlPullParser.h
@@ -60,7 +60,7 @@ public:
     static bool skipCurrentElement(XmlPullParser* parser);
     static bool isGoodEvent(Event event);
 
-    XmlPullParser(std::istream& in);
+    explicit XmlPullParser(std::istream& in);
     ~XmlPullParser();
 
     /**