Rework GoStructInitializationInspection

fixes #2819
go-lang-plugin-org · Dec 8, 2016 · 63d8b74 · 63d8b74
1 parent eb669fc
commit 63d8b74
Show file tree

Hide file tree

Showing 47 changed files with 643 additions and 95 deletions.
diff --git a/src/com/goide/completion/GoStructLiteralCompletion.java b/src/com/goide/completion/GoStructLiteralCompletion.java
@@ -19,9 +19,7 @@
 import com.goide.psi.*;
 import com.goide.psi.impl.GoPsiImplUtil;
 import com.intellij.psi.PsiElement;
-import com.intellij.util.ObjectUtils;
 import com.intellij.util.containers.ContainerUtil;
-import org.jetbrains.annotations.Contract;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
@@ -61,8 +59,8 @@ enum Variants {
 
   @NotNull
   static Variants allowedVariants(@Nullable GoReferenceExpression structFieldReference) {
-    GoValue value = parent(structFieldReference, GoValue.class);
-    GoElement element = parent(value, GoElement.class);
+    GoValue value = GoPsiTreeUtil.getDirectParentOfType(structFieldReference, GoValue.class);
+    GoElement element = GoPsiTreeUtil.getDirectParentOfType(value, GoElement.class);
     if (element != null && element.getKey() != null) {
       return Variants.NONE;
     }
@@ -75,7 +73,7 @@ static Variants allowedVariants(@Nullable GoReferenceExpression structFieldRefer
     boolean hasValueInitializers = false;
     boolean hasFieldValueInitializers = false;
 
-    GoLiteralValue literalValue = parent(element, GoLiteralValue.class);
+    GoLiteralValue literalValue = GoPsiTreeUtil.getDirectParentOfType(element, GoLiteralValue.class);
     List<GoElement> fieldInitializers = literalValue != null ? literalValue.getElementList() : Collections.emptyList();
     for (GoElement initializer : fieldInitializers) {
       if (initializer == element) {
@@ -105,9 +103,4 @@ static Set<String> alreadyAssignedFields(@Nullable GoLiteralValue literal) {
       return identifier != null ? identifier.getText() : null;
     });
   }
-
-  @Contract("null,_->null")
-  private static <T> T parent(@Nullable PsiElement of, @NotNull Class<T> parentClass) {
-    return ObjectUtils.tryCast(of != null ? of.getParent() : null, parentClass);
-  }
 }
diff --git a/src/com/goide/inspections/GoStructInitializationInspection.java b/src/com/goide/inspections/GoStructInitializationInspection.java
@@ -22,94 +22,180 @@
 import com.goide.util.GoUtil;
 import com.intellij.codeInspection.*;
 import com.intellij.codeInspection.ui.SingleCheckboxOptionsPanel;
-import com.intellij.openapi.progress.ProgressManager;
 import com.intellij.openapi.project.Project;
+import com.intellij.openapi.util.Comparing;
 import com.intellij.openapi.util.InvalidDataException;
 import com.intellij.openapi.util.WriteExternalException;
 import com.intellij.psi.PsiElement;
 import com.intellij.psi.util.PsiTreeUtil;
-import com.intellij.util.containers.ContainerUtil;
+import com.intellij.util.ObjectUtils;
 import org.jdom.Element;
+import org.jetbrains.annotations.Contract;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
 import javax.swing.*;
 import java.util.List;
 
+import static com.intellij.util.containers.ContainerUtil.*;
+import static java.util.stream.Collectors.toList;
+import static java.util.stream.IntStream.range;
+
 public class GoStructInitializationInspection extends GoInspectionBase {
-  public static final String REPLACE_WITH_NAMED_STRUCT_FIELD_FIX_NAME = "Replace with named struct field";
+  public static final String REPLACE_WITH_NAMED_STRUCT_FIELD_FIX_NAME = "Replace with named struct fields";
+  private static final GoReplaceWithNamedStructFieldQuickFix QUICK_FIX = new GoReplaceWithNamedStructFieldQuickFix();
   public boolean reportLocalStructs;
   /**
-   * @deprecated use reportLocalStructs
+   * @deprecated use {@link #reportLocalStructs}
    */
   @SuppressWarnings("WeakerAccess") public Boolean reportImportedStructs;
 
   @NotNull
   @Override
   protected GoVisitor buildGoVisitor(@NotNull ProblemsHolder holder, @NotNull LocalInspectionToolSession session) {
     return new GoVisitor() {
+
       @Override
-      public void visitLiteralValue(@NotNull GoLiteralValue o) {
-        if (PsiTreeUtil.getParentOfType(o, GoReturnStatement.class, GoShortVarDeclaration.class, GoAssignmentStatement.class) == null) {
-          return;
-        }
-        PsiElement parent = o.getParent();
-        GoType refType = GoPsiImplUtil.getLiteralType(parent, false);
-        if (refType instanceof GoStructType) {
-          processStructType(holder, o, (GoStructType)refType);
-        }
+      public void visitLiteralValue(@NotNull GoLiteralValue literalValue) {
+        GoStructType structType = getLiteralStructType(literalValue);
+        if (structType == null || !isStructImportedOrLocalAllowed(structType, literalValue)) return;
+
+        List<GoElement> elements = literalValue.getElementList();
+        List<String> keys = getKeys(elements);
+        if (!areKeysMatchesDefinitions(keys, getFieldDefinitionsNames(structType))) return;
+        registerProblemsForElementsWithoutKeys(elements, keys, holder);
       }
     };
   }
 
-  @Override
-  public JComponent createOptionsPanel() {
-    return new SingleCheckboxOptionsPanel("Report for local type definitions as well", this, "reportLocalStructs");
+  @Contract("null -> null")
+  private static GoStructType getLiteralStructType(@Nullable GoLiteralValue literalValue) {
+    GoCompositeLit parentLit = GoPsiTreeUtil.getDirectParentOfType(literalValue, GoCompositeLit.class);
+    if (parentLit != null && !isStructLit(parentLit)) return null;
+
+    GoStructType litType = ObjectUtils.tryCast(GoPsiImplUtil.getLiteralType(literalValue, parentLit == null), GoStructType.class);
+    String definitionName = getFieldDefinitionName(GoPsiTreeUtil.getDirectParentOfType(literalValue, GoValue.class));
+    return definitionName != null && litType != null ? getFieldDefinitionType(litType, definitionName) : litType;
   }
 
-  private void processStructType(@NotNull ProblemsHolder holder, @NotNull GoLiteralValue element, @NotNull GoStructType structType) {
-    if (reportLocalStructs || !GoUtil.inSamePackage(structType.getContainingFile(), element.getContainingFile())) {
-      processLiteralValue(holder, element, structType.getFieldDeclarationList());
-    }
+  @Nullable
+  private static String getFieldDefinitionName(@Nullable GoValue value) {
+    GoKey key = PsiTreeUtil.getChildOfType(GoPsiTreeUtil.getDirectParentOfType(value, GoElement.class), GoKey.class);
+    GoFieldName fieldName = key != null ? key.getFieldName() : null;
+    return fieldName != null ? fieldName.getText() : null;
   }
 
-  private static void processLiteralValue(@NotNull ProblemsHolder holder,
-                                          @NotNull GoLiteralValue o,
-                                          @NotNull List<GoFieldDeclaration> fields) {
-    List<GoElement> vals = o.getElementList();
-    for (int elemId = 0; elemId < vals.size(); elemId++) {
-      ProgressManager.checkCanceled();
-      GoElement element = vals.get(elemId);
-      if (element.getKey() == null && elemId < fields.size()) {
-        String structFieldName = getFieldName(fields.get(elemId));
-        LocalQuickFix[] fixes = structFieldName != null ? new LocalQuickFix[]{new GoReplaceWithNamedStructFieldQuickFix(structFieldName)}
-                                                        : LocalQuickFix.EMPTY_ARRAY;
-        holder.registerProblem(element, "Unnamed field initialization", ProblemHighlightType.GENERIC_ERROR_OR_WARNING, fixes);
-      }
-    }
+  @Nullable
+  private static GoStructType getFieldDefinitionType(@NotNull GoStructType structType, @NotNull String definitionName) {
+    GoFieldDefinition fieldDefinition = getDefinition(structType, definitionName);
+    if (fieldDefinition != null) return ObjectUtils.tryCast(getUnderlyingType(fieldDefinition), GoStructType.class);
+
+    GoAnonymousFieldDefinition anonymousDefinition = getAnonDefinition(structType, definitionName);
+    return anonymousDefinition != null ? ObjectUtils
+      .tryCast(GoPsiImplUtil.getUnderlyingType(anonymousDefinition.getType()), GoStructType.class) : null;
   }
 
   @Nullable
-  private static String getFieldName(@NotNull GoFieldDeclaration declaration) {
-    List<GoFieldDefinition> list = declaration.getFieldDefinitionList();
-    GoFieldDefinition fieldDefinition = ContainerUtil.getFirstItem(list);
-    return fieldDefinition != null ? fieldDefinition.getIdentifier().getText() : null;
+  private static GoType getUnderlyingType(@NotNull GoFieldDefinition fieldDefinition) {
+    GoType type = fieldDefinition.getGoType(null);
+    return type != null ? GoPsiImplUtil.getUnderlyingType(type) : null;
+  }
+
+  @Nullable
+  private static GoAnonymousFieldDefinition getAnonDefinition(@NotNull GoStructType type, @NotNull String definitionName) {
+    return type.getFieldDeclarationList().stream()
+      .map(GoFieldDeclaration::getAnonymousFieldDefinition)
+      .filter(definition -> definition != null && Comparing.equal(definitionName, definition.getName()))
+      .findAny().orElse(null);
+  }
+
+  @Nullable
+  private static GoFieldDefinition getDefinition(@NotNull GoStructType type, @NotNull String definitionName) {
+    return type.getFieldDeclarationList().stream().flatMap(declaration -> declaration.getFieldDefinitionList().stream())
+      .filter(definition -> Comparing.equal(definition.getName(), definitionName))
+      .findAny().orElse(null);
+  }
+
+
+  private static boolean isStructLit(@NotNull GoCompositeLit parentLit) {
+    GoType type = parentLit.getGoType(null);
+    return type != null && GoPsiImplUtil.getUnderlyingType(type) instanceof GoStructType;
+  }
+
+  private boolean isStructImportedOrLocalAllowed(@NotNull GoStructType structType, @NotNull GoLiteralValue literalValue) {
+    return reportLocalStructs || !GoUtil.inSamePackage(structType.getContainingFile(), literalValue.getContainingFile());
+  }
+
+  @NotNull
+  private static List<String> getKeys(@NotNull List<GoElement> elements) {
+    return map(elements, element -> {
+      GoKey key = element.getKey();
+      return key != null ? key.getText() : null;
+    });
+  }
+
+  private static boolean areKeysMatchesDefinitions(@NotNull List<String> keys, @NotNull List<String> fieldDefinitionsNames) {
+    return range(0, keys.size()).allMatch(i -> isNullOrEqual(keys.get(i), GoPsiImplUtil.getByIndex(fieldDefinitionsNames, i)));
+  }
+
+  @Contract("null, _ -> true")
+  private static boolean isNullOrEqual(@Nullable Object o, @Nullable Object objectToCompare) {
+    return o == null || Comparing.equal(o, objectToCompare);
+  }
+
+  @NotNull
+  private static List<String> getFieldDefinitionsNames(@Nullable GoStructType type) {
+    return type != null ? type.getFieldDeclarationList().stream()
+      .flatMap(declaration -> getFieldDefinitionsNames(declaration).stream())
+      .collect(toList()) : emptyList();
+  }
+
+  @NotNull
+  private static List<String> getFieldDefinitionsNames(@NotNull GoFieldDeclaration declaration) {
+    GoAnonymousFieldDefinition definition = declaration.getAnonymousFieldDefinition();
+    return definition != null ? list(definition.getName()) : map(declaration.getFieldDefinitionList(), GoNamedElement::getName);
+  }
+
+  private static void registerProblemsForElementsWithoutKeys(@NotNull List<GoElement> elements,
+                                                             @NotNull List<String> keys,
+                                                             @NotNull ProblemsHolder holder) {
+    for (int i = 0; i < elements.size(); i++) {
+      if (GoPsiImplUtil.getByIndex(keys, i) != null) continue;
+      holder.registerProblem(elements.get(i), "Unnamed field initializations", ProblemHighlightType.WEAK_WARNING, QUICK_FIX);
+    }
+  }
+
+  @Override
+  public JComponent createOptionsPanel() {
+    return new SingleCheckboxOptionsPanel("Report for local type definitions as well", this, "reportLocalStructs");
   }
 
   private static class GoReplaceWithNamedStructFieldQuickFix extends LocalQuickFixBase {
-    private String myStructField;
 
-    public GoReplaceWithNamedStructFieldQuickFix(@NotNull String structField) {
+    public GoReplaceWithNamedStructFieldQuickFix() {
       super(REPLACE_WITH_NAMED_STRUCT_FIELD_FIX_NAME);
-      myStructField = structField;
     }
 
     @Override
     public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) {
-      PsiElement startElement = descriptor.getStartElement();
-      if (startElement instanceof GoElement) {
-        startElement.replace(GoElementFactory.createLiteralValueElement(project, myStructField, startElement.getText()));
-      }
+      PsiElement element = ObjectUtils.tryCast(descriptor.getStartElement(), GoElement.class);
+      GoLiteralValue literal = element != null && element.isValid() ? PsiTreeUtil.getParentOfType(element, GoLiteralValue.class) : null;
+
+      List<GoElement> elements = literal != null ? literal.getElementList() : emptyList();
+      List<String> fieldDefinitionNames = getFieldDefinitionsNames(getLiteralStructType(literal));
+      if (!areKeysMatchesDefinitions(getKeys(elements), fieldDefinitionNames)) return;
+      addKeysToElements(project, elements, fieldDefinitionNames);
+    }
+  }
+
+  private static void addKeysToElements(@NotNull Project project,
+                                        @NotNull List<GoElement> elements,
+                                        @NotNull List<String> fieldDefinitionNames) {
+    for (int i = 0; i < elements.size(); i++) {
+      GoElement element = elements.get(i);
+      String fieldDefinitionName = GoPsiImplUtil.getByIndex(fieldDefinitionNames, i);
+      GoValue value = fieldDefinitionName != null && element.getKey() == null ? element.getValue() : null;
+      if (value != null) element.replace(GoElementFactory.createLiteralValueElement(project, fieldDefinitionName, value.getText()));
     }
   }
 

diff --git a/src/com/goide/psi/GoPsiTreeUtil.java b/src/com/goide/psi/GoPsiTreeUtil.java
@@ -26,8 +26,10 @@
 import com.intellij.psi.stubs.StubElement;
 import com.intellij.psi.util.PsiTreeUtil;
 import com.intellij.psi.util.PsiUtilCore;
+import com.intellij.util.ObjectUtils;
 import com.intellij.util.SmartList;
 import com.intellij.util.containers.ContainerUtil;
+import org.jetbrains.annotations.Contract;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
@@ -155,5 +157,10 @@ private static PsiElement findNotWhiteSpaceElementAtOffset(@NotNull GoFile file,
     }
     return element;
   }
+
+  @Contract("null,_->null")
+  public static <T> T getDirectParentOfType(@Nullable PsiElement element, @NotNull Class<T> aClass) {
+    return element != null ? ObjectUtils.tryCast(element.getParent(), aClass) : null;
+  }
 }
 
diff --git a/src/com/goide/psi/impl/GoElementFactory.java b/src/com/goide/psi/impl/GoElementFactory.java
@@ -256,7 +256,7 @@ public static GoType createType(@NotNull Project project, @NotNull String text)
     return PsiTreeUtil.findChildOfType(file, GoType.class);
   }
 
-  public static PsiElement createLiteralValueElement(@NotNull Project project, @NotNull String key, @NotNull String value) {
+  public static GoElement createLiteralValueElement(@NotNull Project project, @NotNull String key, @NotNull String value) {
     GoFile file = createFileFromText(project, "package a; var _ = struct { a string } { " + key + ": " + value + " }");
     return PsiTreeUtil.findChildOfType(file, GoElement.class);
   }

diff --git a/src/com/goide/psi/impl/GoPsiImplUtil.java b/src/com/goide/psi/impl/GoPsiImplUtil.java
@@ -563,17 +563,19 @@ public static GoType getLiteralType(@Nullable PsiElement context, boolean consid
       }
       literalValue = literalValue.getParent();
     }
+
     return type;
   }
 
   @Nullable
   public static GoValue getParentGoValue(@NotNull PsiElement element) {
     PsiElement place = element;
-    while ((place = PsiTreeUtil.getParentOfType(place, GoLiteralValue.class)) != null) {
+    do {
       if (place.getParent() instanceof GoValue) {
         return (GoValue)place.getParent();
       }
     }
+    while ((place = PsiTreeUtil.getParentOfType(place, GoLiteralValue.class)) != null);
     return null;
   }
 
@@ -1468,7 +1470,8 @@ public static GoExpression getValue(@NotNull GoConstDefinition definition) {
     return getByIndex(((GoConstSpec)parent).getExpressionList(), index);
   }
 
-  private static <T> T getByIndex(@NotNull List<T> list, int index) {
+  @Nullable
+  public static <T> T getByIndex(@NotNull List<T> list, int index) {
     return 0 <= index && index < list.size() ? list.get(index) : null;
   }
 

diff --git a/testData/inspections/go-struct-initialization/quickFix.go b/testData/inspections/go-struct-initialization/quickFix.go
diff --git a/testData/inspections/struct-initialization/anonField-after.go b/testData/inspections/struct-initialization/anonField-after.go
@@ -0,0 +1,11 @@
+package foo
+
+type S struct {
+	X string
+	string
+	Y int
+}
+func main() {
+	var s S
+	s = S{X: "X", string: "a", Y: 1}
+}
diff --git a/testData/inspections/struct-initialization/anonField.go b/testData/inspections/struct-initialization/anonField.go
@@ -0,0 +1,11 @@
+package foo
+
+type S struct {
+	X string
+	string
+	Y int
+}
+func main() {
+	var s S
+	s = S{<caret><weak_warning descr="Unnamed field initializations">"X"</weak_warning>, <weak_warning descr="Unnamed field initializations">"a"</weak_warning>, Y: 1}
+}
diff --git a/testData/inspections/struct-initialization/exceedElementsCountOnlyUnnamed-after.go b/testData/inspections/struct-initialization/exceedElementsCountOnlyUnnamed-after.go
@@ -0,0 +1,8 @@
+package foo
+
+type S struct {
+	X, Y int
+}
+func main() {
+	s := S{X: 1, Y: 0, 2}
+}
diff --git a/testData/inspections/struct-initialization/exceedElementsCountOnlyUnnamed.go b/testData/inspections/struct-initialization/exceedElementsCountOnlyUnnamed.go
@@ -0,0 +1,8 @@
+package foo
+
+type S struct {
+	X, Y int
+}
+func main() {
+	s := S{<weak_warning descr="Unnamed field initializations"><caret>1</weak_warning>, <weak_warning descr="Unnamed field initializations">0</weak_warning>, <weak_warning descr="Unnamed field initializations">2</weak_warning>}
+}
diff --git a/testData/inspections/struct-initialization/exceedElementsCountWithNamed.go b/testData/inspections/struct-initialization/exceedElementsCountWithNamed.go
@@ -0,0 +1,9 @@
+package foo
+
+type S struct {
+	X, Y int
+}
+func main() {
+	s := S{<caret>1, 0, X: 2}
+
+}