Skip to content

Commit 925c529

Browse files
wbarswbars
committed
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
1 parent c8af522 commit 925c529

15 files changed

+107
-64
lines changed

‎src/com/goide/inspections/GoStructInitializationInspection.java

Lines changed: 38 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
import com.intellij.openapi.util.Comparing;
2727
import com.intellij.openapi.util.InvalidDataException;
2828
import com.intellij.openapi.util.WriteExternalException;
29+
import com.intellij.psi.PsiElement;
30+
import com.intellij.psi.util.PsiTreeUtil;
2931
import com.intellij.util.ObjectUtils;
3032
import org.jdom.Element;
3133
import org.jetbrains.annotations.Contract;
@@ -35,7 +37,6 @@
3537
import javax.swing.*;
3638
import java.util.List;
3739

38-
import static com.intellij.openapi.util.NullUtils.hasNull;
3940
import static com.intellij.util.containers.ContainerUtil.*;
4041
import static java.util.stream.Collectors.toList;
4142
import static java.util.stream.IntStream.range;
@@ -55,23 +56,42 @@ protected GoVisitor buildGoVisitor(@NotNull ProblemsHolder holder, @NotNull Loca
5556
return new GoVisitor() {
5657

5758
@Override
58-
public void visitCompositeLit(@NotNull GoCompositeLit compositeLit) {
59-
GoLiteralValue literalValue = compositeLit.getLiteralValue();
59+
public void visitLiteralValue(@NotNull GoLiteralValue literalValue) {
6060
GoStructType structType = getStructType(literalValue);
6161
if (structType == null || !isStructImportedOrLocalAllowed(structType, literalValue)) return;
6262

63-
List<String> elementsNames = getNames(literalValue.getElementList());
64-
if (hasNull(elementsNames.toArray()) && areElementsNamesMatchesDefinitions(elementsNames, getFieldDefinitionsNames(structType))) {
65-
holder.registerProblem(literalValue, "Unnamed field initializations", ProblemHighlightType.GENERIC_ERROR_OR_WARNING, QUICK_FIX);
66-
}
63+
List<GoElement> elements = literalValue.getElementList();
64+
List<String> elementsNames = getNames(elements);
65+
if (!areElementsNamesMatchesDefinitions(elementsNames, getFieldDefinitionsNames(structType))) return;
66+
67+
zip(elementsNames, elements).forEach(pair -> {
68+
if (pair.first != null) return;
69+
holder.registerProblem(pair.second, "Unnamed field initializations", ProblemHighlightType.WEAK_WARNING, QUICK_FIX);
70+
});
6771
}
6872
};
6973
}
7074

7175
@Nullable
7276
@Contract("null -> null")
73-
private static GoStructType getStructType(@Nullable GoLiteralValue literal) {
74-
return literal != null ? ObjectUtils.tryCast(GoPsiImplUtil.getLiteralType(literal.getParent(), false), GoStructType.class) : null;
77+
private static GoStructType getStructType(@Nullable GoLiteralValue literalValue) {
78+
GoCompositeLit lit = PsiTreeUtil.getParentOfType(literalValue, GoCompositeLit.class);
79+
GoType litType = lit != null ? lit.getType() : null;
80+
return isSliceElementValue(literalValue, litType)
81+
? getStructType((GoArrayOrSliceType)litType)
82+
: ObjectUtils.tryCast(GoPsiImplUtil.getLiteralType(lit, false), GoStructType.class);
83+
}
84+
85+
@Contract("_, null -> false; null, _ -> false")
86+
private static boolean isSliceElementValue(@Nullable GoLiteralValue literalValue, @Nullable GoType parentLitType) {
87+
return parentLitType instanceof GoArrayOrSliceType && literalValue != null && literalValue.getParent() instanceof GoValue;
88+
}
89+
90+
@Nullable
91+
@Contract("null -> null")
92+
private static GoStructType getStructType(@Nullable GoArrayOrSliceType slice) {
93+
GoType type = slice != null ? slice.getType() : null;
94+
return type != null ? ObjectUtils.tryCast(type.getUnderlyingType(), GoStructType.class) : null;
7595
}
7696

7797
private boolean isStructImportedOrLocalAllowed(@NotNull GoStructType structType, @NotNull GoLiteralValue literalValue) {
@@ -88,19 +108,15 @@ private static List<String> getNames(@NotNull List<GoElement> elements) {
88108

89109
private static boolean areElementsNamesMatchesDefinitions(@NotNull List<String> elementsNames,
90110
@NotNull List<String> fieldDefinitionsNames) {
91-
return range(0, elementsNames.size()).allMatch(i -> isNullOrEqual(elementsNames.get(i), getByIndex(fieldDefinitionsNames, i)));
111+
return range(0, elementsNames.size())
112+
.allMatch(i -> isNullOrEqual(elementsNames.get(i), GoPsiImplUtil.getByIndex(fieldDefinitionsNames, i)));
92113
}
93114

94115
@Contract("null, _ -> true")
95116
private static boolean isNullOrEqual(@Nullable Object o, @Nullable Object objectToCompare) {
96117
return o == null || Comparing.equal(o, objectToCompare);
97118
}
98119

99-
@Nullable
100-
private static String getByIndex(@NotNull List<String> list, int index) {
101-
return 0 <= index && index < list.size() ? list.get(index) : null;
102-
}
103-
104120
@NotNull
105121
private static List<String> getFieldDefinitionsNames(@NotNull GoStructType type) {
106122
return type.getFieldDeclarationList().stream()
@@ -127,21 +143,22 @@ public GoReplaceWithNamedStructFieldQuickFix() {
127143

128144
@Override
129145
public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) {
130-
GoLiteralValue literal = ObjectUtils.tryCast(descriptor.getStartElement(), GoLiteralValue.class);
146+
PsiElement element = ObjectUtils.tryCast(descriptor.getStartElement(), GoElement.class);
147+
GoLiteralValue literal = element != null && element.isValid() ? PsiTreeUtil.getParentOfType(element, GoLiteralValue.class) : null;
131148
GoStructType structType = getStructType(literal);
132149
List<GoElement> elements = structType != null ? literal.getElementList() : emptyList();
133150
List<String> fieldDefinitionNames = structType != null ? getFieldDefinitionsNames(structType) : emptyList();
134151
if (!areElementsNamesMatchesDefinitions(getNames(elements), fieldDefinitionNames)) return;
135-
replaceElementsByNamed(elements, fieldDefinitionNames, project);
152+
replaceElementsByNamed(project, elements, fieldDefinitionNames);
136153
}
137154
}
138155

139-
private static void replaceElementsByNamed(@NotNull List<GoElement> elements,
140-
@NotNull List<String> fieldDefinitionNames,
141-
@NotNull Project project) {
156+
private static void replaceElementsByNamed(@NotNull Project project,
157+
@NotNull List<GoElement> elements,
158+
@NotNull List<String> fieldDefinitionNames) {
142159
for (int i = 0; i < elements.size(); i++) {
143160
GoElement element = elements.get(i);
144-
String fieldDefinitionName = getByIndex(fieldDefinitionNames, i);
161+
String fieldDefinitionName = GoPsiImplUtil.getByIndex(fieldDefinitionNames, i);
145162
GoValue value = fieldDefinitionName != null && element.getKey() == null ? element.getValue() : null;
146163
if (value == null) continue;
147164

‎src/com/goide/psi/impl/GoPsiImplUtil.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1468,7 +1468,8 @@ public static GoExpression getValue(@NotNull GoConstDefinition definition) {
14681468
return getByIndex(((GoConstSpec)parent).getExpressionList(), index);
14691469
}
14701470

1471-
private static <T> T getByIndex(@NotNull List<T> list, int index) {
1471+
@Nullable
1472+
public static <T> T getByIndex(@NotNull List<T> list, int index) {
14721473
return 0 <= index && index < list.size() ? list.get(index) : null;
14731474
}
14741475

‎testData/inspections/struct-initialization/anonField.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,5 @@ type S struct {
77
}
88
func main() {
99
var s S
10-
s = S<weak_warning descr="Unnamed field initializations">{<caret>"X", "a", Y: 1}</weak_warning>
10+
s = S{<caret><weak_warning descr="Unnamed field initializations">"X"</weak_warning>, <weak_warning descr="Unnamed field initializations">"a"</weak_warning>, Y: 1}
1111
}

‎testData/inspections/struct-initialization/exceedElementsCountOnlyUnnamed.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,5 @@ type S struct {
44
X, Y int
55
}
66
func main() {
7-
s := S<weak_warning descr="Unnamed field initializations">{<caret>1, 0, 2}</weak_warning>
7+
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>}
88
}

‎testData/inspections/struct-initialization/innerAnonStruct.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,6 @@ func main() {
1111
Z int
1212
}
1313

14-
s := S<weak_warning descr="Unnamed field initializations">{1<caret>, B{Y: 2}, 3}</weak_warning>
14+
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>}
1515
print(s.B.Y)
1616
}

‎testData/inspections/struct-initialization/innerStruct.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,6 @@ func main() {
1111
Z int
1212
}
1313

14-
s := S<weak_warning descr="Unnamed field initializations">{1<caret>, B{Y: 2}}</weak_warning>
14+
s := S{<weak_warning descr="Unnamed field initializations">1<caret></weak_warning>, <weak_warning descr="Unnamed field initializations">B{Y: 2}</weak_warning>}
1515
print(s.b.Y)
1616
}

‎testData/inspections/struct-initialization/literalValue-after.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
package foo
2+
3+
func main() {
4+
type B struct {
5+
string
6+
}
7+
type S struct {
8+
int
9+
B
10+
}
11+
_ = []S{ {int: 1, B: {string: "a"}}}
12+
}

‎testData/inspections/struct-initialization/literalValue.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
package foo
2+
3+
func main() {
4+
type B struct {
5+
string
6+
}
7+
type S struct {
8+
int
9+
B
10+
}
11+
_ = []S{ {<weak_warning descr="Unnamed field initializations"><caret>1</weak_warning>, B: {string: "a"}}}
12+
}

‎testData/inspections/struct-initialization/multipleFieldsOneline.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ type S struct {
55
}
66
func main() {
77
var s S
8-
s = S<weak_warning descr="Unnamed field initializations">{0, 0}</weak_warning>
9-
s = S<weak_warning descr="Unnamed field initializations">{X: 0, 0}</weak_warning>
10-
s = S<weak_warning descr="Unnamed field initializations">{0, Y: 0}</weak_warning>
8+
s = S{<weak_warning descr="Unnamed field initializations">0</weak_warning>, <weak_warning descr="Unnamed field initializations">0</weak_warning>}
9+
s = S{X: 0, <weak_warning descr="Unnamed field initializations">0</weak_warning>}
10+
s = S{<weak_warning descr="Unnamed field initializations">0</weak_warning>, Y: 0}
1111
}

‎testData/inspections/struct-initialization/onelineQuickfix.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,5 @@ type S struct {
55
}
66
func main() {
77
var s S
8-
s = S<weak_warning descr="Unnamed field initializations">{<caret>0, 0}</weak_warning>
8+
s = S{<caret><weak_warning descr="Unnamed field initializations">0</weak_warning>, <weak_warning descr="Unnamed field initializations">0</weak_warning>}
99
}

‎testData/inspections/struct-initialization/quickFix.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ package foo
33
import "io"
44

55
func _() {
6-
_ = io.LimitedReader<weak_warning descr="Unnamed field initializations">{
7-
<caret>nil,
8-
}</weak_warning>
6+
_ = io.LimitedReader{
7+
<caret><weak_warning descr="Unnamed field initializations">nil</weak_warning>,
8+
}
99
}

‎testData/inspections/struct-initialization/uninitializedStructImportedOnly.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -113,15 +113,15 @@ func _() {
113113
}
114114

115115
// bad defs
116-
_ = io.LimitedReader<weak_warning descr="Unnamed field initializations">{
117-
nil,
118-
}</weak_warning>
119-
_ = os.LinkError<weak_warning descr="Unnamed field initializations">{
120-
"string",
121-
"string",
122-
"string",
123-
nil,
124-
}</weak_warning>
116+
_ = io.LimitedReader{
117+
<weak_warning descr="Unnamed field initializations">nil</weak_warning>,
118+
}
119+
_ = os.LinkError{
120+
<weak_warning descr="Unnamed field initializations">"string"</weak_warning>,
121+
<weak_warning descr="Unnamed field initializations">"string"</weak_warning>,
122+
<weak_warning descr="Unnamed field initializations">"string"</weak_warning>,
123+
<weak_warning descr="Unnamed field initializations">nil</weak_warning>,
124+
}
125125
}
126126

127127
type assertion struct {

‎testData/inspections/struct-initialization/uninitializedStructWithLocal.go

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -76,21 +76,21 @@ func _() struct {
7676
_ = demo{a: "demo", b: 1}
7777

7878
// bad defs
79-
_ = demo<weak_warning descr="Unnamed field initializations">{"demo"}</weak_warning>
80-
b, _ := demo<weak_warning descr="Unnamed field initializations">{"demo"}</weak_warning>, 1
81-
_ = demo<weak_warning descr="Unnamed field initializations">{
82-
"demo",
83-
1,
84-
"demo",
85-
}</weak_warning>
86-
87-
_ = demo<weak_warning descr="Unnamed field initializations">{
79+
_ = demo{<weak_warning descr="Unnamed field initializations">"demo"</weak_warning>}
80+
b, _ := demo{<weak_warning descr="Unnamed field initializations">"demo"</weak_warning>}, 1
81+
_ = demo{
82+
<weak_warning descr="Unnamed field initializations">"demo"</weak_warning>,
83+
<weak_warning descr="Unnamed field initializations">1</weak_warning>,
84+
<weak_warning descr="Unnamed field initializations">"demo"</weak_warning>,
85+
}
86+
87+
_ = demo{
8888
a: "demo",
89-
1,
90-
}</weak_warning>
89+
<weak_warning descr="Unnamed field initializations">1</weak_warning>,
90+
}
9191
_ = bemo{x: "demo"}
9292
_ = b
93-
return struct{ x string }<weak_warning descr="Unnamed field initializations">{"demo"}</weak_warning>
93+
return struct{ x string }{<weak_warning descr="Unnamed field initializations">"demo"</weak_warning>}
9494
}
9595

9696
type Item struct {
@@ -113,15 +113,15 @@ func _() {
113113
}
114114

115115
// bad defs
116-
_ = io.LimitedReader<weak_warning descr="Unnamed field initializations">{
117-
nil,
118-
}</weak_warning>
119-
_ = os.LinkError<weak_warning descr="Unnamed field initializations">{
120-
"string",
121-
"string",
122-
"string",
123-
nil,
124-
}</weak_warning>
116+
_ = io.LimitedReader{
117+
<weak_warning descr="Unnamed field initializations">nil</weak_warning>,
118+
}
119+
_ = os.LinkError{
120+
<weak_warning descr="Unnamed field initializations">"string"</weak_warning>,
121+
<weak_warning descr="Unnamed field initializations">"string"</weak_warning>,
122+
<weak_warning descr="Unnamed field initializations">"string"</weak_warning>,
123+
<weak_warning descr="Unnamed field initializations">nil</weak_warning>,
124+
}
125125
}
126126

127127
type assertion struct {

‎testData/inspections/struct-initialization/varDeclaration.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,5 @@ type S struct {
44
X, Y int
55
}
66
func main() {
7-
var s S = S<weak_warning descr="Unnamed field initializations">{<caret>0, 0}</weak_warning>
7+
var s S = S{<caret><weak_warning descr="Unnamed field initializations">0</weak_warning>, <weak_warning descr="Unnamed field initializations">0</weak_warning>}
88
}

‎tests/com/goide/inspections/GoStructInitializationInspectionTest.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ protected void tearDown() throws Exception {
5151
public void testInnerStruct() { doQuickfixTest(); }
5252
public void testOnelineQuickfix() { doQuickfixTest(); }
5353
public void testVarDeclaration() { doQuickfixTest(); }
54+
public void testLiteralValue() { doQuickfixTest(); }
5455

5556

5657
private long doHighlightTest(boolean allowLocalStructs) {

0 commit comments

Comments
 (0)