Review marks:

- report each element with null key instead of literal - handle case with literal as slice element value - made GoPsiImplUtil#getByIndex public - some extra checks and refactoring
go-lang-plugin-org · Nov 21, 2016 · 0179518 · 0179518
1 parent c8af522
commit 0179518
Show file tree

Hide file tree

Showing 15 changed files with 107 additions and 64 deletions.
diff --git a/src/com/goide/inspections/GoStructInitializationInspection.java b/src/com/goide/inspections/GoStructInitializationInspection.java
@@ -26,6 +26,8 @@
 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.ObjectUtils;
 import org.jdom.Element;
 import org.jetbrains.annotations.Contract;
@@ -35,7 +37,6 @@
 import javax.swing.*;
 import java.util.List;
 
-import static com.intellij.openapi.util.NullUtils.hasNull;
 import static com.intellij.util.containers.ContainerUtil.*;
 import static java.util.stream.Collectors.toList;
 import static java.util.stream.IntStream.range;
@@ -55,23 +56,42 @@ protected GoVisitor buildGoVisitor(@NotNull ProblemsHolder holder, @NotNull Loca
     return new GoVisitor() {
 
       @Override
-      public void visitCompositeLit(@NotNull GoCompositeLit compositeLit) {
-        GoLiteralValue literalValue = compositeLit.getLiteralValue();
+      public void visitLiteralValue(@NotNull GoLiteralValue literalValue) {
         GoStructType structType = getStructType(literalValue);
         if (structType == null || !isStructImportedOrLocalAllowed(structType, literalValue)) return;
 
-        List<String> elementsNames = getNames(literalValue.getElementList());
-        if (hasNull(elementsNames.toArray()) && areElementsNamesMatchesDefinitions(elementsNames, getFieldDefinitionsNames(structType))) {
-          holder.registerProblem(literalValue, "Unnamed field initializations", ProblemHighlightType.GENERIC_ERROR_OR_WARNING, QUICK_FIX);
-        }
+        List<GoElement> elements = literalValue.getElementList();
+        List<String> elementsNames = getNames(elements);
+        if (!areElementsNamesMatchesDefinitions(elementsNames, getFieldDefinitionsNames(structType))) return;
+
+        zip(elementsNames, elements).forEach(pair -> {
+          if (pair.first != null) return;
+          holder.registerProblem(pair.second, "Unnamed field initializations", ProblemHighlightType.WEAK_WARNING, QUICK_FIX);
+        });
       }
     };
   }
 
   @Nullable
   @Contract("null -> null")
-  private static GoStructType getStructType(@Nullable GoLiteralValue literal) {
-    return literal != null ? ObjectUtils.tryCast(GoPsiImplUtil.getLiteralType(literal.getParent(), false), GoStructType.class) : null;
+  private static GoStructType getStructType(@Nullable GoLiteralValue literalValue) {
+    GoCompositeLit lit = literalValue != null ? PsiTreeUtil.getParentOfType(literalValue, GoCompositeLit.class) : null;
+    GoType litType = lit != null ? lit.getType() : null;
+    return isSliceElementValue(literalValue, litType)
+           ? getStructType((GoArrayOrSliceType)litType)
+           : ObjectUtils.tryCast(GoPsiImplUtil.getLiteralType(lit, false), GoStructType.class);
+  }
+
+  @Contract("_, null -> false; null, _ -> false")
+  private static boolean isSliceElementValue(@Nullable GoLiteralValue literalValue, @Nullable GoType parentType) {
+    return parentType instanceof GoArrayOrSliceType && literalValue != null && literalValue.getParent() instanceof GoValue;
+  }
+
+  @Nullable
+  @Contract("null -> null")
+  private static GoStructType getStructType(@Nullable GoArrayOrSliceType slice) {
+    GoType type = slice != null ? slice.getType() : null;
+    return type != null ? ObjectUtils.tryCast(type.getUnderlyingType(), GoStructType.class) : null;
   }
 
   private boolean isStructImportedOrLocalAllowed(@NotNull GoStructType structType, @NotNull GoLiteralValue literalValue) {
@@ -88,19 +108,15 @@ private static List<String> getNames(@NotNull List<GoElement> elements) {
 
   private static boolean areElementsNamesMatchesDefinitions(@NotNull List<String> elementsNames,
                                                             @NotNull List<String> fieldDefinitionsNames) {
-    return range(0, elementsNames.size()).allMatch(i -> isNullOrEqual(elementsNames.get(i), getByIndex(fieldDefinitionsNames, i)));
+    return range(0, elementsNames.size())
+      .allMatch(i -> isNullOrEqual(elementsNames.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);
   }
 
-  @Nullable
-  private static String getByIndex(@NotNull List<String> list, int index) {
-    return 0 <= index && index < list.size() ? list.get(index) : null;
-  }
-
   @NotNull
   private static List<String> getFieldDefinitionsNames(@NotNull GoStructType type) {
     return type.getFieldDeclarationList().stream()
@@ -127,21 +143,22 @@ public GoReplaceWithNamedStructFieldQuickFix() {
 
     @Override
     public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) {
-      GoLiteralValue literal = ObjectUtils.tryCast(descriptor.getStartElement(), GoLiteralValue.class);
+      PsiElement element = ObjectUtils.tryCast(descriptor.getStartElement(), GoElement.class);
+      GoLiteralValue literal = element != null && element.isValid() ? PsiTreeUtil.getParentOfType(element, GoLiteralValue.class) : null;
       GoStructType structType = getStructType(literal);
       List<GoElement> elements = structType != null ? literal.getElementList() : emptyList();
       List<String> fieldDefinitionNames = structType != null ? getFieldDefinitionsNames(structType) : emptyList();
       if (!areElementsNamesMatchesDefinitions(getNames(elements), fieldDefinitionNames)) return;
-      replaceElementsByNamed(elements, fieldDefinitionNames, project);
+      replaceElementsByNamed(project, elements, fieldDefinitionNames);
     }
   }
 
-  private static void replaceElementsByNamed(@NotNull List<GoElement> elements,
-                                             @NotNull List<String> fieldDefinitionNames,
-                                             @NotNull Project project) {
+  private static void replaceElementsByNamed(@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 = getByIndex(fieldDefinitionNames, i);
+      String fieldDefinitionName = GoPsiImplUtil.getByIndex(fieldDefinitionNames, i);
       GoValue value = fieldDefinitionName != null && element.getKey() == null ? element.getValue() : null;
       if (value == null) continue;
 

diff --git a/src/com/goide/psi/impl/GoPsiImplUtil.java b/src/com/goide/psi/impl/GoPsiImplUtil.java
@@ -1468,7 +1468,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/struct-initialization/anonField.go b/testData/inspections/struct-initialization/anonField.go
@@ -7,5 +7,5 @@ type S struct {
 }
 func main() {
 	var s S
-	s = S<weak_warning descr="Unnamed field initializations">{<caret>"X", "a", Y: 1}</weak_warning>
+	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.go b/testData/inspections/struct-initialization/exceedElementsCountOnlyUnnamed.go
@@ -4,5 +4,5 @@ type S struct {
 	X, Y int
 }
 func main() {
-	s := S<weak_warning descr="Unnamed field initializations">{<caret>1, 0, 2}</weak_warning>
+	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/innerAnonStruct.go b/testData/inspections/struct-initialization/innerAnonStruct.go
@@ -11,6 +11,6 @@ func main() {
 		Z int
 	}
 
-	s := S<weak_warning descr="Unnamed field initializations">{1<caret>, B{Y: 2}, 3}</weak_warning>
+	s := S{<weak_warning descr="Unnamed field initializations">1<caret></weak_warning>, <weak_warning descr="Unnamed field initializations">B{Y: 2}</weak_warning>, <weak_warning descr="Unnamed field initializations">3</weak_warning>}
 	print(s.B.Y)
 }
diff --git a/testData/inspections/struct-initialization/innerStruct.go b/testData/inspections/struct-initialization/innerStruct.go
@@ -11,6 +11,6 @@ func main() {
 		Z int
 	}
 
-	s := S<weak_warning descr="Unnamed field initializations">{1<caret>, B{Y: 2}}</weak_warning>
+	s := S{<weak_warning descr="Unnamed field initializations">1<caret></weak_warning>, <weak_warning descr="Unnamed field initializations">B{Y: 2}</weak_warning>}
 	print(s.b.Y)
 }
diff --git a/testData/inspections/struct-initialization/literalValue-after.go b/testData/inspections/struct-initialization/literalValue-after.go
@@ -0,0 +1,12 @@
+package foo
+
+func main() {
+	type B struct {
+		string
+	}
+	type S struct {
+		int
+		B
+	}
+	_ = []S{ {int: 1, B: {string: "a"}}}
+}
diff --git a/testData/inspections/struct-initialization/literalValue.go b/testData/inspections/struct-initialization/literalValue.go
@@ -0,0 +1,12 @@
+package foo
+
+func main() {
+	type B struct {
+		string
+	}
+	type S struct {
+		int
+		B
+	}
+	_ = []S{ {<weak_warning descr="Unnamed field initializations"><caret>1</weak_warning>, B: {string: "a"}}}
+}
diff --git a/testData/inspections/struct-initialization/multipleFieldsOneline.go b/testData/inspections/struct-initialization/multipleFieldsOneline.go
@@ -5,7 +5,7 @@ type S struct {
 }
 func main() {
 	var s S
-	s = S<weak_warning descr="Unnamed field initializations">{0, 0}</weak_warning>
-	s = S<weak_warning descr="Unnamed field initializations">{X: 0, 0}</weak_warning>
-	s = S<weak_warning descr="Unnamed field initializations">{0, Y: 0}</weak_warning>
+	s = S{<weak_warning descr="Unnamed field initializations">0</weak_warning>, <weak_warning descr="Unnamed field initializations">0</weak_warning>}
+	s = S{X: 0, <weak_warning descr="Unnamed field initializations">0</weak_warning>}
+	s = S{<weak_warning descr="Unnamed field initializations">0</weak_warning>, Y: 0}
 }
diff --git a/testData/inspections/struct-initialization/onelineQuickfix.go b/testData/inspections/struct-initialization/onelineQuickfix.go
@@ -5,5 +5,5 @@ type S struct {
 }
 func main() {
 	var s S
-	s = S<weak_warning descr="Unnamed field initializations">{<caret>0, 0}</weak_warning>
+	s = S{<caret><weak_warning descr="Unnamed field initializations">0</weak_warning>, <weak_warning descr="Unnamed field initializations">0</weak_warning>}
 }
diff --git a/testData/inspections/struct-initialization/quickFix.go b/testData/inspections/struct-initialization/quickFix.go
@@ -3,7 +3,7 @@ package foo
 import "io"
 
 func _() {
-	_ = io.LimitedReader<weak_warning descr="Unnamed field initializations">{
-		<caret>nil,
-	}</weak_warning>
+	_ = io.LimitedReader{
+		<caret><weak_warning descr="Unnamed field initializations">nil</weak_warning>,
+	}
 }
diff --git a/testData/inspections/struct-initialization/uninitializedStructImportedOnly.go b/testData/inspections/struct-initialization/uninitializedStructImportedOnly.go
@@ -113,15 +113,15 @@ func _() {
 	}
 
 	// bad defs
-	_ = io.LimitedReader<weak_warning descr="Unnamed field initializations">{
-		nil,
-	}</weak_warning>
-	_ = os.LinkError<weak_warning descr="Unnamed field initializations">{
-		"string",
-		"string",
-		"string",
-		nil,
-	}</weak_warning>
+	_ = io.LimitedReader{
+	<weak_warning descr="Unnamed field initializations">nil</weak_warning>,
+	}
+	_ = os.LinkError{
+		<weak_warning descr="Unnamed field initializations">"string"</weak_warning>,
+		<weak_warning descr="Unnamed field initializations">"string"</weak_warning>,
+		<weak_warning descr="Unnamed field initializations">"string"</weak_warning>,
+		<weak_warning descr="Unnamed field initializations">nil</weak_warning>,
+	}
 }
 
 type assertion struct {

diff --git a/testData/inspections/struct-initialization/uninitializedStructWithLocal.go b/testData/inspections/struct-initialization/uninitializedStructWithLocal.go
@@ -76,21 +76,21 @@ func _() struct {
 	_ = demo{a: "demo", b: 1}
 
 	// bad defs
-	_ = demo<weak_warning descr="Unnamed field initializations">{"demo"}</weak_warning>
-	b, _ := demo<weak_warning descr="Unnamed field initializations">{"demo"}</weak_warning>, 1
-	_ = demo<weak_warning descr="Unnamed field initializations">{
-		"demo",
-		1,
-		"demo",
-	}</weak_warning>
-
-	_ = demo<weak_warning descr="Unnamed field initializations">{
+	_ = demo{<weak_warning descr="Unnamed field initializations">"demo"</weak_warning>}
+	b, _ := demo{<weak_warning descr="Unnamed field initializations">"demo"</weak_warning>}, 1
+	_ = demo{
+		<weak_warning descr="Unnamed field initializations">"demo"</weak_warning>,
+		<weak_warning descr="Unnamed field initializations">1</weak_warning>,
+		<weak_warning descr="Unnamed field initializations">"demo"</weak_warning>,
+	}
+
+	_ = demo{
 		a: "demo",
-		1,
-	}</weak_warning>
+		<weak_warning descr="Unnamed field initializations">1</weak_warning>,
+	}
 	_ = bemo{x: "demo"}
 	_ = b
-	return struct{ x string }<weak_warning descr="Unnamed field initializations">{"demo"}</weak_warning>
+	return struct{ x string }{<weak_warning descr="Unnamed field initializations">"demo"</weak_warning>}
 }
 
 type Item struct {
@@ -113,15 +113,15 @@ func _() {
 	}
 
 	// bad defs
-	_ = io.LimitedReader<weak_warning descr="Unnamed field initializations">{
-		nil,
-	}</weak_warning>
-	_ = os.LinkError<weak_warning descr="Unnamed field initializations">{
-		"string",
-		"string",
-		"string",
-		nil,
-	}</weak_warning>
+	_ = io.LimitedReader{
+		<weak_warning descr="Unnamed field initializations">nil</weak_warning>,
+	}
+	_ = os.LinkError{
+		<weak_warning descr="Unnamed field initializations">"string"</weak_warning>,
+		<weak_warning descr="Unnamed field initializations">"string"</weak_warning>,
+		<weak_warning descr="Unnamed field initializations">"string"</weak_warning>,
+		<weak_warning descr="Unnamed field initializations">nil</weak_warning>,
+	}
 }
 
 type assertion struct {

diff --git a/testData/inspections/struct-initialization/varDeclaration.go b/testData/inspections/struct-initialization/varDeclaration.go
@@ -4,5 +4,5 @@ type S struct {
 	X, Y int
 }
 func main() {
-	var s S = S<weak_warning descr="Unnamed field initializations">{<caret>0, 0}</weak_warning>
+	var s S = S{<caret><weak_warning descr="Unnamed field initializations">0</weak_warning>, <weak_warning descr="Unnamed field initializations">0</weak_warning>}
 }
diff --git a/tests/com/goide/inspections/GoStructInitializationInspectionTest.java b/tests/com/goide/inspections/GoStructInitializationInspectionTest.java
@@ -51,6 +51,7 @@ protected void tearDown() throws Exception {
   public void testInnerStruct()                     { doQuickfixTest(); }
   public void testOnelineQuickfix()                 { doQuickfixTest(); }
   public void testVarDeclaration()                  { doQuickfixTest(); }
+  public void testLiteralValue()                    { doQuickfixTest(); }
 
 
   private long doHighlightTest(boolean allowLocalStructs) {