Add AIDL Lint check for redundant oneway

Check for redundant oneway on methods inside a oneway interface.
"oneway interface A { oneway void b(); }"
Method b() is oneway already since it's part of a oneway interface.

Bug: 221322034
Test: atest --host aidl_unittests
Change-Id: I563eec585d8db1a36d4264b032c801bfa398e674

5 files changed, 56 insertions, 1 deletions

diff --git a/aidl_language.cpp b/aidl_language.cpp
index 67208181..06af9074 100644
--- a/aidl_language.cpp
+++ b/aidl_language.cpp
@@ -1097,6 +1097,7 @@ AidlMethod::AidlMethod(const AidlLocation& location, bool oneway, AidlTypeSpecif
                        const Comments& comments, int id)
     : AidlMember(location, comments),
       oneway_(oneway),
+      oneway_annotation_(oneway),
       type_(type),
       name_(name),
       arguments_(std::move(*args)),
@@ -1707,7 +1708,7 @@ bool AidlUnionDecl::CheckValid(const AidlTypenames& typenames) const {
 AidlInterface::AidlInterface(const AidlLocation& location, const std::string& name,
                              const Comments& comments, bool oneway, const std::string& package,
                              std::vector<std::unique_ptr<AidlMember>>* members)
-    : AidlDefinedType(location, name, comments, package, members) {
+    : AidlDefinedType(location, name, comments, package, members), oneway_annotation_(oneway) {
   for (auto& m : GetMethods()) {
     m.get()->ApplyInterfaceOneway(oneway);
   }
diff --git a/aidl_language.h b/aidl_language.h
index 3b1c77d1..f963edb5 100644
--- a/aidl_language.h
+++ b/aidl_language.h
@@ -878,6 +878,7 @@ class AidlMethod : public AidlMember {
   // set if this method is part of an interface that is marked oneway
   void ApplyInterfaceOneway(bool oneway) { oneway_ = oneway_ || oneway; }
   bool IsOneway() const { return oneway_; }
+  bool HasOnewayAnnotation() const { return oneway_annotation_; }
 
   const std::string& GetName() const { return name_; }
   bool HasId() const { return has_id_; }
@@ -917,7 +918,11 @@ class AidlMethod : public AidlMember {
   void DispatchVisit(AidlVisitor& v) const override { v.Visit(*this); }
 
  private:
+  // oneway_ may be set by the method or the parent interface. If the interface is oneway,
+  // is also oneway. oneway_annotation_ may only be set on creation, and may not be overridden
+  // by the parent interface. It is used to detect redundant oneway annotations.
   bool oneway_;
+  bool oneway_annotation_;
   std::unique_ptr<AidlTypeSpecifier> type_;
   std::string name_;
   const std::vector<std::unique_ptr<AidlArgument>> arguments_;
@@ -1215,6 +1220,10 @@ class AidlInterface final : public AidlDefinedType {
   bool UsesPermissions() const;
   std::string GetDescriptor() const;
   void DispatchVisit(AidlVisitor& v) const override { v.Visit(*this); }
+  bool HasOnewayAnnotation() const { return oneway_annotation_; }
+
+ private:
+  bool oneway_annotation_;
 };
 
 inline std::string SimpleName(const std::string& qualified_name) {
diff --git a/diagnostics.cpp b/diagnostics.cpp
index 5112173b..5c51f03d 100644
--- a/diagnostics.cpp
+++ b/diagnostics.cpp
@@ -224,6 +224,31 @@ struct DiagnoseMixedOneway : DiagnosticsVisitor {
   }
 };
 
+struct DiagnoseRedundantOneway : DiagnosticsVisitor {
+  DiagnoseRedundantOneway(DiagnosticsContext& diag) : DiagnosticsVisitor(diag) {}
+  void Visit(const AidlInterface& i) override {
+    if (i.HasOnewayAnnotation()) {
+      for (const auto& m : i.GetMethods()) {
+        if (!m->IsUserDefined()) continue;
+        if (Suppressed(*m)) continue;
+        if (m->HasOnewayAnnotation()) {
+          diag.Report(i.GetLocation(), DiagnosticID::redundant_oneway)
+              << "The interface '" << i.GetName()
+              << "' is oneway. Redundant oneway annotation for method '" << m->GetName() << "'.";
+        }
+      }
+    }
+  }
+  bool Suppressed(const AidlMethod& m) const {
+    for (const auto& w : m.GetType().SuppressWarnings()) {
+      if (w == to_string(DiagnosticID::redundant_oneway)) {
+        return true;
+      }
+    }
+    return false;
+  }
+};
+
 struct DiagnoseOutArray : DiagnosticsVisitor {
   DiagnoseOutArray(DiagnosticsContext& diag) : DiagnosticsVisitor(diag) {}
   void Visit(const AidlMethod& m) override {
@@ -435,6 +460,7 @@ bool Diagnose(const AidlDocument& doc, const DiagnosticMapping& mapping) {
   DiagnoseUntypedCollection{diag}.Check(doc);
   DiagnosePermissionAnnotations{diag}.Check(doc);
   DiagnoseRedundantNames{diag}.Check(doc);
+  DiagnoseRedundantOneway{diag}.Check(doc);
 
   return diag.ErrorCount() == 0;
 }
diff --git a/diagnostics.inc b/diagnostics.inc
index 85b52033..9abdee88 100644
--- a/diagnostics.inc
+++ b/diagnostics.inc
@@ -12,3 +12,4 @@ DIAG(unique_import, "unique-import", false)
 DIAG(unknown_warning, "unknown-warning", false)
 DIAG(untyped_collection, "untyped-collection", false)
 DIAG(redundant_name, "redundant-name", false)
+DIAG(redundant_oneway, "redundant-oneway", false)
diff --git a/diagnostics_unittest.cpp b/diagnostics_unittest.cpp
index dcbb7011..feb8189f 100644
--- a/diagnostics_unittest.cpp
+++ b/diagnostics_unittest.cpp
@@ -157,6 +157,24 @@ TEST_F(DiagnosticsTest, OnewayInterfaceIsOkayWithSyntheticMethods) {
   });
 }
 
+TEST_F(DiagnosticsTest, RedundantOnewayMethodAnnotationInOnewayInterface) {
+  expect_diagnostic = DiagnosticID::redundant_oneway;
+  ParseFiles({
+      {"IFoo.aidl", "oneway interface IFoo { oneway void foo(int a); }"},
+  });
+}
+
+TEST_F(DiagnosticsTest, RedundantOnewayMethodSuppressedAtMethod) {
+  enable_diagnostic = DiagnosticID::redundant_oneway;
+  expect_diagnostic = {};
+  ParseFiles({
+      {"IFoo.aidl",
+       "oneway interface IFoo {\n"
+       "  @SuppressWarnings(value={\"redundant-oneway\"}) oneway void bar();\n"
+       "}"},
+  });
+}
+
 TEST_F(DiagnosticsTest, ArraysAsOutputParametersConsideredHarmful) {
   expect_diagnostic = DiagnosticID::out_array;
   ParseFiles({