From 804743e62928e81d899fd9d1fc9ef4c343e5cce4 Mon Sep 17 00:00:00 2001
From: Yaroslav <saturn4er@gmail.com>
Date: Wed, 3 Feb 2021 22:27:18 +0200
Subject: [PATCH] Some improvements (#103)

* Validate method ids and names

* Refactor struct fields validation

* Add thrift context

* Validate duplicate method names
---
 .../plugins/thrift/ThriftContext.java         |  18 ++++
 .../ThriftDuplicatesInspection.java           | 102 +++++++++++++-----
 thrift/src/main/resources/META-INF/plugin.xml |   2 +-
 3 files changed, 93 insertions(+), 29 deletions(-)
 create mode 100644 thrift/src/main/java/com/intellij/plugins/thrift/ThriftContext.java

diff --git a/thrift/src/main/java/com/intellij/plugins/thrift/ThriftContext.java b/thrift/src/main/java/com/intellij/plugins/thrift/ThriftContext.java
new file mode 100644
index 00000000..a5dd3cf1
--- /dev/null
+++ b/thrift/src/main/java/com/intellij/plugins/thrift/ThriftContext.java
@@ -0,0 +1,18 @@
+package com.intellij.plugins.thrift;
+
+import com.intellij.codeInsight.template.TemplateActionContext;
+import com.intellij.codeInsight.template.TemplateContextType;
+import org.jetbrains.annotations.NotNull;
+
+public class ThriftContext extends TemplateContextType {
+
+  protected ThriftContext() {
+    super("THRIFT", "Thrift");
+  }
+
+  @Override
+  public boolean isInContext(@NotNull TemplateActionContext templateActionContext) {
+    return templateActionContext.getFile().getName().endsWith(".thrift");
+  }
+
+}
\ No newline at end of file
diff --git a/thrift/src/main/java/com/intellij/plugins/thrift/inspections/ThriftDuplicatesInspection.java b/thrift/src/main/java/com/intellij/plugins/thrift/inspections/ThriftDuplicatesInspection.java
index 50bc52f8..8980bc99 100644
--- a/thrift/src/main/java/com/intellij/plugins/thrift/inspections/ThriftDuplicatesInspection.java
+++ b/thrift/src/main/java/com/intellij/plugins/thrift/inspections/ThriftDuplicatesInspection.java
@@ -55,39 +55,47 @@ public void visitTopLevelDeclaration(@NotNull ThriftTopLevelDeclaration o) {
         if (topIdentifier != null && !topLevelNames.add(topIdentifier.getText())) {
           // Repeated top level names
           result.add(manager.createProblemDescriptor(
-            topIdentifier,
-            getDisplayName(),
-            true,
-            ProblemHighlightType.ERROR,
-            isOnTheFly
+                  topIdentifier,
+                  getDisplayName(),
+                  true,
+                  ProblemHighlightType.ERROR,
+                  isOnTheFly
           ));
         }
 
-        Set<String> fieldNames = new HashSet<String>();
-        Set<String> fieldIds = new HashSet<String>();
-        for (ThriftDeclaration d : o.findSubDeclarations()) {
-          ThriftDefinitionName identifier = d.getIdentifier();
-          if (identifier != null && !fieldNames.add(identifier.getText())) {
-            // Repeated field names
-            result.add(manager.createProblemDescriptor(
-              identifier,
-              getDisplayName(),
-              true,
-              ProblemHighlightType.ERROR,
-              isOnTheFly
-            ));
+        if (o instanceof ThriftService) {
+          ThriftService service = (ThriftService) o;
+          ThriftServiceBody body = service.getServiceBody();
+
+          if (body != null) {
+            Set<String> methodNames = new HashSet<String>();
+
+            for (ThriftFunction f : body.getFunctionList()) {
+              String methodName = f.getDefinitionName().getName();
+
+              if(!methodNames.add(methodName)){
+                result.add(manager.createProblemDescriptor(
+                        f.getIdentifier(),
+                        String.format("multiple methods with name '%s'", methodName),
+                        true,
+                        ProblemHighlightType.ERROR,
+                        isOnTheFly
+                ));
+              }
+
+              result.addAll(checkFieldList(manager, isOnTheFly, f.getFieldList(), "args"));
+              ThriftThrows t = f.getThrows();
+              if (t != null) {
+                result.addAll(checkFieldList(manager, isOnTheFly, t.getFieldList(), "throws"));
+              }
+            }
           }
+        }
 
-          ThriftFieldID fieldID = PsiTreeUtil.getChildOfType(d, ThriftFieldID.class);
-          if (fieldID != null && !fieldIds.add(fieldID.getText())) {
-            //Reapted fieldIDs
-            result.add(manager.createProblemDescriptor(
-              fieldID,
-              getDisplayName(),
-              true,
-              ProblemHighlightType.ERROR,
-              isOnTheFly
-            ));
+        if (o instanceof ThriftStruct) {
+          ThriftFields fields = ((ThriftStruct) o).getFields();
+          if (fields != null) {
+            result.addAll(checkFieldList(manager, isOnTheFly, fields.getFieldList(), "fields"));
           }
         }
       }
@@ -99,4 +107,42 @@ public void visitElement(@NotNull PsiElement element) {
     }.visitFile(file);
     return ArrayUtil.toObjectArray(result, ProblemDescriptor.class);
   }
+
+  private List<ProblemDescriptor> checkFieldList(
+          @NotNull final InspectionManager manager,
+          final boolean isOnTheFly,
+          List<ThriftField> fields,
+          String part
+  ) {
+    Set<String> names = new HashSet<String>();
+    Set<String> ids = new HashSet<String>();
+    final List<ProblemDescriptor> result = new ArrayList<ProblemDescriptor>();
+
+    for (ThriftField field : fields) {
+      ThriftFieldID id = field.getFieldID();
+      ThriftDefinitionName name = field.getDefinitionName();
+
+      if (id != null && !ids.add(id.getText())) {
+        result.add(manager.createProblemDescriptor(
+                field.getIdentifier(),
+                String.format("multiple %s with id %s", part, id.getIntConstant().getText()),
+                true,
+                ProblemHighlightType.ERROR,
+                isOnTheFly
+        ));
+      }
+
+      if (name != null && !names.add(name.getText())) {
+        result.add(manager.createProblemDescriptor(
+                field.getIdentifier(),
+                String.format("multiple %s with name '%s'", part, name.getText()),
+                true,
+                ProblemHighlightType.ERROR,
+                isOnTheFly
+        ));
+      }
+    }
+
+    return result;
+  }
 }
diff --git a/thrift/src/main/resources/META-INF/plugin.xml b/thrift/src/main/resources/META-INF/plugin.xml
index db22f4fc..24295438 100644
--- a/thrift/src/main/resources/META-INF/plugin.xml
+++ b/thrift/src/main/resources/META-INF/plugin.xml
@@ -120,7 +120,6 @@
   <depends>com.intellij.modules.vcs</depends>
   <depends>com.intellij.modules.xml</depends>
   <depends>com.intellij.modules.xdebugger</depends>
-  <depends>com.intellij.modules.java</depends>
 
   <application-components>
     <!-- Add your application components here -->
@@ -150,6 +149,7 @@
     <lang.commenter language="thrift" implementationClass="com.intellij.plugins.thrift.ThriftCommenter"/>
     <lang.implementationTextSelectioner language="thrift"
                                         implementationClass="com.intellij.plugins.thrift.ThriftImplementationTextSelectioner"/>
+    <liveTemplateContext implementation="com.intellij.plugins.thrift.ThriftContext"/>
 
     <!--<colorSettingsPage/>-->
     <!--<codeStyleSettingsProvider/>-->