commit f886f98554afaaadab92f3025f67a96e3e4d21f1
author Jooyung Han <jooyung@google.com> Fri Jun 11 09:56:04 2021 +0900
committer Jooyung Han <jooyung@google.com> Wed Jun 16 00:01:05 2021 +0000
tree 4c2f4b7ffaca3f76a956d7fe69ef034df79a995b
parent 6529b82782a76e66e33f3a4eb325c03eab27a1f1

warning: import should introduce unique simple name

Importing two different types with the same simple name is not allowed.

- collision with other imports

  import q.Bar;
  import r.Bar; // error

- collision with top-level type decl's name

  import q.Bar; // error
  parcelable Bar { .. }

Bug: 190782365
Test: aidl_unittests
Test: m
Change-Id: I13c0bde3f710d8230c61c1f0bb9680ea32f980eb

aidl_language.h

diff --git a/aidl_language.h b/aidl_language.h
index deb530a..77c8d0a 100644
--- a/aidl_language.h
+++ b/aidl_language.h
@@ -1193,6 +1193,7 @@
   AidlImport& operator=(AidlImport&&) = delete;
 
   const std::string& GetNeededClass() const { return needed_class_; }
+  std::string SimpleName() const { return needed_class_.substr(needed_class_.rfind('.') + 1); }
   void TraverseChildren(std::function<void(const AidlNode&)>) const {}
   void DispatchVisit(AidlVisitor& v) const { v.Visit(*this); }
 

diagnostics.cpp

diff --git a/diagnostics.cpp b/diagnostics.cpp
index 694e96a..ce30db5 100644
--- a/diagnostics.cpp
+++ b/diagnostics.cpp
@@ -248,6 +248,34 @@
   }
 };
 
+struct DiagnoseImports : DiagnosticsVisitor {
+  DiagnoseImports(DiagnosticsContext& diag) : DiagnosticsVisitor(diag) {}
+  void Visit(const AidlDocument& doc) override {
+    auto collide_with_decls = [&](const auto& import) {
+      for (const auto& type : doc.DefinedTypes()) {
+        if (type->GetCanonicalName() != import->GetNeededClass() &&
+            type->GetName() == import->SimpleName()) {
+          return true;
+        }
+      }
+      return false;
+    };
+
+    std::set<std::string> imported_names;
+    for (const auto& import : doc.Imports()) {
+      if (collide_with_decls(import)) {
+        diag.Report(import->GetLocation(), DiagnosticID::unique_import)
+            << import->SimpleName() << " is already defined in this file.";
+      }
+      auto [_, inserted] = imported_names.insert(import->SimpleName());
+      if (!inserted) {
+        diag.Report(import->GetLocation(), DiagnosticID::unique_import)
+            << import->SimpleName() << " is already imported.";
+      }
+    }
+  }
+};
+
 bool Diagnose(const AidlDocument& doc, const DiagnosticMapping& mapping) {
   DiagnosticsContext diag(mapping);
 
@@ -259,6 +287,7 @@
   DiagnoseOutArray{diag}.Check(doc);
   DiagnoseFileDescriptor{diag}.Check(doc);
   DiagnoseOutNullable{diag}.Check(doc);
+  DiagnoseImports{diag}.Check(doc);
 
   return diag.ErrorCount() == 0;
 }

diagnostics.inc

diff --git a/diagnostics.inc b/diagnostics.inc
index 59ea7c8..c7dd4e5 100644
--- a/diagnostics.inc
+++ b/diagnostics.inc
@@ -7,4 +7,5 @@
 DIAG(mixed_oneway, "mixed-oneway", false)
 DIAG(out_array, "out-array", false)
 DIAG(out_nullable, "out-nullable", false)
+DIAG(unique_import, "unique-import", false)
 DIAG(unknown_warning, "unknown-warning", false)

diagnostics_unittest.cpp

diff --git a/diagnostics_unittest.cpp b/diagnostics_unittest.cpp
index 0ea3141..eed824c 100644
--- a/diagnostics_unittest.cpp
+++ b/diagnostics_unittest.cpp
@@ -178,3 +178,41 @@
                "}"},
               {"Bar.aidl", "parcelable Bar {}"}});
 }
+
+TEST_F(DiagnosticsTest, RejectImportsCollisionWithTopLevelDecl) {
+  expect_diagnostics = {DiagnosticID::unique_import};
+  ParseFiles({{"p/IFoo.aidl",
+               "package p;\n"
+               "import q.IFoo;\n"  // should collide with previous import
+               "interface IFoo{}"},
+              {"q/IFoo.aidl", "package q; interface IFoo{}"}});
+}
+
+TEST_F(DiagnosticsTest, RejectImportsCollision) {
+  expect_diagnostics = {DiagnosticID::unique_import};
+  ParseFiles({{"p/IFoo.aidl",
+               "package p;\n"
+               "import q.IBar;\n"
+               "import r.IBar;\n"  // should collide with previous import
+               "interface IFoo{}"},
+              {"q/IBar.aidl", "package q; interface IBar{}"},
+              {"r/IBar.aidl", "package r; interface IBar{}"}});
+}
+
+TEST_F(DiagnosticsTest, AllowImportingSelf) {
+  expect_diagnostics = {};
+  ParseFiles({{"p/IFoo.aidl",
+               "package p;\n"
+               "import p.IFoo;\n"
+               "interface IFoo{}"}});
+}
+
+TEST_F(DiagnosticsTest, AllowRedundantImports) {
+  expect_diagnostics = {};
+  ParseFiles({{"p/IFoo.aidl",
+               "package p;\n"
+               "import q.IBar;\n"
+               "import q.IBar;\n"  // ugly, but okay
+               "interface IFoo{}"},
+              {"q/IBar.aidl", "package q; interface IBar{}"}});
+}
\ No newline at end of file