From 7640c4f5428473d074e5d2a84233933b189e9c56 Mon Sep 17 00:00:00 2001 From: wannabe Date: Sat, 29 Oct 2016 06:13:25 +1000 Subject: [PATCH 1/3] Move to struct declaration: handle struct declaration via _var_ case Replace single struct declaration with short var declaration, or add to existing struct literal in statement --- ...GoMoveToStructInitializationIntention.java | 190 +++++++++++++----- src/com/goide/psi/impl/GoElementFactory.java | 6 + .../notStructDeclaration.go | 11 + .../parensStructDeclaration-after.go | 11 + .../parensStructDeclaration.go | 11 + .../structDeclaration-after.go | 11 + .../structDeclaration.go | 11 + .../structDeclarationMultipleExpressions.go | 11 + .../structDeclarationWithEmptyInit-after.go | 11 + .../structDeclarationWithEmptyInit.go | 11 + ...structDeclarationWithNonEmptyInit-after.go | 12 ++ .../structDeclarationWithNonEmptyInit.go | 12 ++ .../structDeclarationWrongQualifier.go | 12 ++ ...veToStructInitializationIntentionTest.java | 7 + 14 files changed, 275 insertions(+), 52 deletions(-) create mode 100644 testData/intentions/move-to-struct-initialization/notStructDeclaration.go create mode 100644 testData/intentions/move-to-struct-initialization/parensStructDeclaration-after.go create mode 100644 testData/intentions/move-to-struct-initialization/parensStructDeclaration.go create mode 100644 testData/intentions/move-to-struct-initialization/structDeclaration-after.go create mode 100644 testData/intentions/move-to-struct-initialization/structDeclaration.go create mode 100644 testData/intentions/move-to-struct-initialization/structDeclarationMultipleExpressions.go create mode 100644 testData/intentions/move-to-struct-initialization/structDeclarationWithEmptyInit-after.go create mode 100644 testData/intentions/move-to-struct-initialization/structDeclarationWithEmptyInit.go create mode 100644 testData/intentions/move-to-struct-initialization/structDeclarationWithNonEmptyInit-after.go create mode 100644 testData/intentions/move-to-struct-initialization/structDeclarationWithNonEmptyInit.go create mode 100644 testData/intentions/move-to-struct-initialization/structDeclarationWrongQualifier.go diff --git a/src/com/goide/intentions/GoMoveToStructInitializationIntention.java b/src/com/goide/intentions/GoMoveToStructInitializationIntention.java index 0040439367..b32aaf145f 100644 --- a/src/com/goide/intentions/GoMoveToStructInitializationIntention.java +++ b/src/com/goide/intentions/GoMoveToStructInitializationIntention.java @@ -60,12 +60,32 @@ public boolean isAvailable(@NotNull Project project, Editor editor, @NotNull Psi private static Data getData(@NotNull PsiElement element) { if (!element.isValid() || !element.isWritable()) return null; GoAssignmentStatement assignment = getValidAssignmentParent(element); - GoReferenceExpression selectedFieldReference = assignment != null ? getFieldReferenceExpression(element, assignment) : null; - GoCompositeLit compositeLit = selectedFieldReference != null ? getStructLiteralByReference(selectedFieldReference, assignment) : null; - if (compositeLit == null) return null; + GoStatement previousStatement = assignment != null ? PsiTreeUtil.getPrevSiblingOfType(assignment, GoStatement.class) : null; + GoReferenceExpression selectedFieldReferenceExpression = + previousStatement != null ? getFieldReferenceExpression(element, assignment, previousStatement) : null; + if (selectedFieldReferenceExpression == null) return null; - List references = getUninitializedSingleFieldReferences(assignment, selectedFieldReference, compositeLit); - return !references.isEmpty() ? new Data(assignment, compositeLit, references) : null; + GoVarDefinition structDefinition = getDefinition(selectedFieldReferenceExpression); + boolean needReplaceDeclarationWithShortVar = isUnassigned(getSingleVarSpecByDefinition(previousStatement, structDefinition)); + + GoCompositeLit compositeLit = getStructLiteralByReference(selectedFieldReferenceExpression, previousStatement); + if (compositeLit == null && !needReplaceDeclarationWithShortVar || structDefinition == null) return null; + + List references = + getUninitializedSingleFieldReferences(assignment, previousStatement, structDefinition, compositeLit); + return !references.isEmpty() ? new Data(assignment, compositeLit, references, previousStatement, structDefinition) : null; + } + + @Nullable + private static GoCompositeLit createStructLiteral(@NotNull GoVarDefinition definition, @NotNull Project project) { + GoType type = definition.getGoType(null); + return type != null ? GoElementFactory.createCompositeLit(project, type) : null; + } + + @Nullable + @Contract("null -> null") + private static GoVarDefinition getDefinition(@Nullable GoReferenceExpression referenceExpressions) { + return ObjectUtils.tryCast(resolveQualifier(referenceExpressions), GoVarDefinition.class); } @Nullable @@ -77,17 +97,22 @@ private static GoAssignmentStatement getValidAssignmentParent(@Nullable PsiEleme @Nullable private static GoReferenceExpression getFieldReferenceExpression(@NotNull PsiElement selectedElement, - @NotNull GoAssignmentStatement assignment) { + @NotNull GoAssignmentStatement assignment, + @NotNull GoStatement previousStatement) { GoReferenceExpression selectedReferenceExpression = PsiTreeUtil.getTopmostParentOfType(selectedElement, GoReferenceExpression.class); if (isFieldReferenceExpression(selectedReferenceExpression)) { - return !isAssignedInPreviousStatement(selectedReferenceExpression, assignment) ? selectedReferenceExpression : null; + return !isAssignedInStatement(getRightExpression(selectedReferenceExpression, assignment), previousStatement) + ? selectedReferenceExpression : null; } List fieldReferenceExpressions = getFieldReferenceExpressions(assignment); - if (exists(fieldReferenceExpressions, expression -> isAssignedInPreviousStatement(expression, assignment))) return null; + if (exists(fieldReferenceExpressions, + expression -> isAssignedInStatement(getRightExpression(expression, assignment), previousStatement))) { + return null; + } - Set resolvedFields = map2Set(fieldReferenceExpressions, GoMoveToStructInitializationIntention::resolveQualifier); - return resolvedFields.size() == 1 ? getFirstItem(fieldReferenceExpressions) : null; + Set resolvedQualifiers = map2Set(fieldReferenceExpressions, GoMoveToStructInitializationIntention::resolveQualifier); + return resolvedQualifiers.size() == 1 ? getFirstItem(fieldReferenceExpressions) : null; } @NotNull @@ -97,11 +122,27 @@ private static List getFieldReferenceExpressions(@NotNull } @Nullable - private static GoReferenceExpression unwrapParensAndCast(@Nullable PsiElement e) { - while (e instanceof GoParenthesesExpr) { - e = ((GoParenthesesExpr)e).getExpression(); + private static GoReferenceExpression unwrapParensAndCast(@Nullable PsiElement element) { + while (element instanceof GoParenthesesExpr) { + element = ((GoParenthesesExpr)element).getExpression(); } - return ObjectUtils.tryCast(e, GoReferenceExpression.class); + return ObjectUtils.tryCast(element, GoReferenceExpression.class); + } + + @Nullable + @Contract("_, null -> null; null, _ -> null") + private static GoVarSpec getSingleVarSpecByDefinition(@Nullable GoStatement statement, + @Nullable GoVarDefinition definition) { + GoVarDeclaration declaration = statement != null ? statement.getVarDeclaration() : null; + List varSpecs = declaration != null ? declaration.getVarSpecList() : emptyList(); + GoVarSpec singleVarSpec = varSpecs.size() == 1 ? getFirstItem(varSpecs) : null; + List varDefinitions = singleVarSpec != null ? singleVarSpec.getVarDefinitionList() : emptyList(); + return varDefinitions.size() == 1 && definition == getFirstItem(varDefinitions) ? singleVarSpec : null; + } + + @Contract("null -> false") + private static boolean isUnassigned(@Nullable GoVarSpec varSpec) { + return varSpec != null && varSpec.getExpressionList().isEmpty(); } @Contract("null -> false") @@ -114,14 +155,16 @@ private static boolean isFieldDefinition(@Nullable PsiElement element) { return element instanceof GoFieldDefinition || element instanceof GoAnonymousFieldDefinition; } - private static boolean isAssignedInPreviousStatement(@NotNull GoExpression referenceExpression, - @NotNull GoAssignmentStatement assignment) { - GoReferenceExpression rightExpression = - unwrapParensAndCast(GoPsiImplUtil.getRightExpression(assignment, getTopmostExpression(referenceExpression))); + private static boolean isAssignedInStatement(@Nullable GoReferenceExpression referenceExpression, + @NotNull GoStatement statement) { + PsiElement resolve = referenceExpression != null ? referenceExpression.resolve() : null; + return exists(getLeftHandElements(statement), element -> isResolvedTo(element, resolve)); + } - PsiElement resolve = rightExpression != null ? rightExpression.resolve() : null; - GoStatement previousElement = resolve != null ? PsiTreeUtil.getPrevSiblingOfType(assignment, GoStatement.class) : null; - return previousElement != null && exists(getLeftHandElements(previousElement), e -> isResolvedTo(e, resolve)); + @Nullable + private static GoReferenceExpression getRightExpression(@NotNull GoExpression expression, + @NotNull GoAssignmentStatement assignment) { + return unwrapParensAndCast(GoPsiImplUtil.getRightExpression(assignment, getTopmostExpression(expression))); } @NotNull @@ -129,35 +172,35 @@ private static GoExpression getTopmostExpression(@NotNull GoExpression expressio return ObjectUtils.notNull(PsiTreeUtil.getTopmostParentOfType(expression, GoExpression.class), expression); } - private static boolean isResolvedTo(@Nullable PsiElement e, @Nullable PsiElement resolve) { - if (e instanceof GoVarDefinition) return resolve == e; + private static boolean isResolvedTo(@Nullable PsiElement element, @Nullable PsiElement resolve) { + if (element instanceof GoVarDefinition) return resolve == element; - GoReferenceExpression refExpression = unwrapParensAndCast(e); + GoReferenceExpression refExpression = unwrapParensAndCast(element); return refExpression != null && refExpression.resolve() == resolve; } @NotNull private static List getUninitializedSingleFieldReferences(@NotNull GoAssignmentStatement assignment, - @NotNull GoReferenceExpression fieldReferenceExpression, - @NotNull GoCompositeLit compositeLit) { - PsiElement resolve = resolveQualifier(fieldReferenceExpression); + @NotNull GoStatement previousStatement, + @Nullable GoVarDefinition definition, + @Nullable GoCompositeLit compositeLit) { List uninitializedFieldReferencesByQualifier = - filter(getUninitializedFieldReferenceExpressions(assignment, compositeLit), e -> isResolvedTo(e.getQualifier(), resolve)); + filter(getUninitializedFieldReferenceExpressions(assignment, compositeLit, previousStatement), + element -> isResolvedTo(element.getQualifier(), definition)); MultiMap resolved = groupBy(uninitializedFieldReferencesByQualifier, GoReferenceExpression::resolve); return map(filter(resolved.entrySet(), set -> set.getValue().size() == 1), set -> getFirstItem(set.getValue())); } @Nullable private static GoCompositeLit getStructLiteralByReference(@NotNull GoReferenceExpression fieldReferenceExpression, - @NotNull GoAssignmentStatement assignment) { - GoStatement previousStatement = PsiTreeUtil.getPrevSiblingOfType(assignment, GoStatement.class); - if (previousStatement instanceof GoSimpleStatement) { - return getStructLiteral(fieldReferenceExpression, (GoSimpleStatement)previousStatement); + @NotNull GoStatement statement) { + if (statement instanceof GoSimpleStatement) { + return getStructLiteral(fieldReferenceExpression, (GoSimpleStatement)statement); } - if (previousStatement instanceof GoAssignmentStatement) { - return getStructLiteral(fieldReferenceExpression, (GoAssignmentStatement)previousStatement); + if (statement instanceof GoAssignmentStatement) { + return getStructLiteral(fieldReferenceExpression, (GoAssignmentStatement)statement); } - return null; + return getStructLiteral(getDefinition(fieldReferenceExpression), statement); } @Nullable @@ -172,15 +215,16 @@ private static GoCompositeLit getStructLiteral(@NotNull GoReferenceExpression fi } @Nullable - private static PsiElement resolveQualifier(@NotNull GoReferenceExpression fieldReferenceExpression) { - GoReferenceExpression qualifier = fieldReferenceExpression.getQualifier(); + @Contract("null -> null") + private static PsiElement resolveQualifier(@Nullable GoReferenceExpression fieldReferenceExpression) { + GoReferenceExpression qualifier = fieldReferenceExpression != null ? fieldReferenceExpression.getQualifier() : null; return qualifier != null ? qualifier.resolve() : null; } @Nullable private static GoCompositeLit getStructLiteral(@NotNull GoReferenceExpression fieldReferenceExpression, @NotNull GoAssignmentStatement structAssignment) { - GoVarDefinition varDefinition = ObjectUtils.tryCast(resolveQualifier(fieldReferenceExpression), GoVarDefinition.class); + GoVarDefinition varDefinition = getDefinition(fieldReferenceExpression); PsiElement field = fieldReferenceExpression.resolve(); if (varDefinition == null || !isFieldDefinition(field) || !hasStructTypeWithField(varDefinition, (GoNamedElement)field)) { return null; @@ -193,6 +237,13 @@ private static GoCompositeLit getStructLiteral(@NotNull GoReferenceExpression fi return ObjectUtils.tryCast(compositeLit, GoCompositeLit.class); } + @Nullable + @Contract("null, _ -> null") + private static GoCompositeLit getStructLiteral(@Nullable GoVarDefinition definition, @NotNull GoStatement statement) { + GoVarSpec varSpec = definition != null ? getSingleVarSpecByDefinition(statement, definition) : null; + return varSpec != null ? ObjectUtils.tryCast(getFirstItem(varSpec.getRightExpressionsList()), GoCompositeLit.class) : null; + } + private static boolean hasStructTypeWithField(@NotNull GoVarDefinition structVarDefinition, @NotNull GoNamedElement field) { GoType type = structVarDefinition.getGoType(null); GoStructType structType = type != null ? ObjectUtils.tryCast(type.getUnderlyingType(), GoStructType.class) : null; @@ -207,15 +258,19 @@ private static boolean isFieldInitialization(@NotNull GoElement element, @NotNul @NotNull private static List getUninitializedFieldReferenceExpressions(@NotNull GoAssignmentStatement assignment, - @NotNull GoCompositeLit structLiteral) { - return filter(getFieldReferenceExpressions(assignment), expression -> - isUninitializedFieldReferenceExpression(expression, structLiteral) && !isAssignedInPreviousStatement(expression, assignment)); + @Nullable GoCompositeLit structLiteral, + @NotNull GoStatement previousStatement) { + return filter(getFieldReferenceExpressions(assignment), + expression -> isUninitializedFieldReferenceExpression(expression, structLiteral) && + !isAssignedInStatement(getRightExpression(expression, assignment), previousStatement)); } - @Contract("null, _-> false") + @Contract("null, _-> false; !null, null -> true") private static boolean isUninitializedFieldReferenceExpression(@Nullable GoReferenceExpression fieldReferenceExpression, - @NotNull GoCompositeLit structLiteral) { + @Nullable GoCompositeLit structLiteral) { if (fieldReferenceExpression == null) return false; + if (structLiteral == null) return true; + GoLiteralValue literalValue = structLiteral.getLiteralValue(); PsiElement resolve = fieldReferenceExpression.resolve(); return literalValue != null && isFieldDefinition(resolve) && @@ -238,19 +293,29 @@ private static List getLeftHandElements(@NotNull GoStateme public void invoke(@NotNull Project project, Editor editor, @NotNull PsiElement element) throws IncorrectOperationException { Data data = getData(element); if (data == null) return; - moveFieldReferenceExpressions(data); + + boolean needReplaceDeclarationWithShortVar = data.getCompositeLit() == null; + GoCompositeLit compositeLit = + needReplaceDeclarationWithShortVar ? createStructLiteral(data.getStructDefinition(), project) : data.getCompositeLit(); + if (compositeLit == null || needReplaceDeclarationWithShortVar && data.getStructDeclaration() == null) return; + + moveFieldReferenceExpressions(data.getReferenceExpressions(), compositeLit.getLiteralValue(), data.getAssignment()); + if (!needReplaceDeclarationWithShortVar) return; + GoStatement shortVarStatement = + GoElementFactory.createShortVarDeclarationStatement(project, data.getStructDefinition().getText(), compositeLit.getText()); + data.getStructDeclaration().replace(shortVarStatement); } - private static void moveFieldReferenceExpressions(@NotNull Data data) { - GoLiteralValue literalValue = data.getCompositeLit().getLiteralValue(); + private static void moveFieldReferenceExpressions(@NotNull List referenceExpressions, + @Nullable GoLiteralValue literalValue, + @NotNull GoAssignmentStatement parentAssignment) { if (literalValue == null) return; - - for (GoReferenceExpression expression : data.getReferenceExpressions()) { + for (GoReferenceExpression expression : referenceExpressions) { GoExpression anchor = getTopmostExpression(expression); - GoExpression fieldValue = GoPsiImplUtil.getRightExpression(data.getAssignment(), anchor); + GoExpression fieldValue = GoPsiImplUtil.getRightExpression(parentAssignment, anchor); if (fieldValue == null) continue; - GoPsiImplUtil.deleteExpressionFromAssignment(data.getAssignment(), anchor); + GoPsiImplUtil.deleteExpressionFromAssignment(parentAssignment, anchor); addFieldDefinition(literalValue, expression.getIdentifier().getText(), fieldValue.getText()); } } @@ -272,25 +337,46 @@ private static class Data { private final GoCompositeLit myCompositeLit; private final GoAssignmentStatement myAssignment; private final List myReferenceExpressions; + private final GoStatement myStructDeclaration; + private final GoVarDefinition myStructDefinition; public Data(@NotNull GoAssignmentStatement assignment, - @NotNull GoCompositeLit compositeLit, - @NotNull List referenceExpressions) { + @Nullable GoCompositeLit compositeLit, + @NotNull List referenceExpressions, + @Nullable GoStatement structDeclaration, + @NotNull GoVarDefinition structDefinition) { myCompositeLit = compositeLit; myAssignment = assignment; myReferenceExpressions = referenceExpressions; + myStructDeclaration = structDeclaration; + myStructDefinition = structDefinition; } + @Nullable public GoCompositeLit getCompositeLit() { return myCompositeLit; } + @NotNull public GoAssignmentStatement getAssignment() { return myAssignment; } + @NotNull public List getReferenceExpressions() { return myReferenceExpressions; } + + @Nullable + public GoStatement getStructDeclaration() { + return myStructDeclaration; + } + + @NotNull + public GoVarDefinition getStructDefinition() { + return myStructDefinition; + } } } + + diff --git a/src/com/goide/psi/impl/GoElementFactory.java b/src/com/goide/psi/impl/GoElementFactory.java index a9c4939ad2..d81feb2471 100644 --- a/src/com/goide/psi/impl/GoElementFactory.java +++ b/src/com/goide/psi/impl/GoElementFactory.java @@ -266,4 +266,10 @@ public static GoTypeDeclaration createTypeDeclaration(@NotNull Project project, GoFile file = createFileFromText(project, "package a; type " + name + " " + type.getText()); return PsiTreeUtil.findChildOfType(file, GoTypeDeclaration.class); } + + @NotNull + public static GoCompositeLit createCompositeLit(@NotNull Project project, @NotNull GoType type) { + GoFile file = createFileFromText(project, "package a; var _ = " + type.getText() + "{}"); + return PsiTreeUtil.findChildOfType(file, GoCompositeLit.class); + } } \ No newline at end of file diff --git a/testData/intentions/move-to-struct-initialization/notStructDeclaration.go b/testData/intentions/move-to-struct-initialization/notStructDeclaration.go new file mode 100644 index 0000000000..03cfe90a2a --- /dev/null +++ b/testData/intentions/move-to-struct-initialization/notStructDeclaration.go @@ -0,0 +1,11 @@ +package main + +type S struct { + foo string +} + +func main() { + var s string + s.foo = "bar" + print(s.foo) +} \ No newline at end of file diff --git a/testData/intentions/move-to-struct-initialization/parensStructDeclaration-after.go b/testData/intentions/move-to-struct-initialization/parensStructDeclaration-after.go new file mode 100644 index 0000000000..18f6832ca8 --- /dev/null +++ b/testData/intentions/move-to-struct-initialization/parensStructDeclaration-after.go @@ -0,0 +1,11 @@ +package main + +type S struct { + foo string +} + +func main() { + s := S{foo: "bar"} + + print(s.foo) +} \ No newline at end of file diff --git a/testData/intentions/move-to-struct-initialization/parensStructDeclaration.go b/testData/intentions/move-to-struct-initialization/parensStructDeclaration.go new file mode 100644 index 0000000000..5f78e6c2fd --- /dev/null +++ b/testData/intentions/move-to-struct-initialization/parensStructDeclaration.go @@ -0,0 +1,11 @@ +package main + +type S struct { + foo string +} + +func main() { + var (s S) + s.foo = "bar" + print(s.foo) +} \ No newline at end of file diff --git a/testData/intentions/move-to-struct-initialization/structDeclaration-after.go b/testData/intentions/move-to-struct-initialization/structDeclaration-after.go new file mode 100644 index 0000000000..18f6832ca8 --- /dev/null +++ b/testData/intentions/move-to-struct-initialization/structDeclaration-after.go @@ -0,0 +1,11 @@ +package main + +type S struct { + foo string +} + +func main() { + s := S{foo: "bar"} + + print(s.foo) +} \ No newline at end of file diff --git a/testData/intentions/move-to-struct-initialization/structDeclaration.go b/testData/intentions/move-to-struct-initialization/structDeclaration.go new file mode 100644 index 0000000000..a5ac8a3ba4 --- /dev/null +++ b/testData/intentions/move-to-struct-initialization/structDeclaration.go @@ -0,0 +1,11 @@ +package main + +type S struct { + foo string +} + +func main() { + var s S + s.foo = "bar" + print(s.foo) +} \ No newline at end of file diff --git a/testData/intentions/move-to-struct-initialization/structDeclarationMultipleExpressions.go b/testData/intentions/move-to-struct-initialization/structDeclarationMultipleExpressions.go new file mode 100644 index 0000000000..525fe58a61 --- /dev/null +++ b/testData/intentions/move-to-struct-initialization/structDeclarationMultipleExpressions.go @@ -0,0 +1,11 @@ +package main + +type S struct { + foo string +} + +func main() { + var s, b S + s.foo = "bar" + print(s.foo) +} \ No newline at end of file diff --git a/testData/intentions/move-to-struct-initialization/structDeclarationWithEmptyInit-after.go b/testData/intentions/move-to-struct-initialization/structDeclarationWithEmptyInit-after.go new file mode 100644 index 0000000000..6a7a17315e --- /dev/null +++ b/testData/intentions/move-to-struct-initialization/structDeclarationWithEmptyInit-after.go @@ -0,0 +1,11 @@ +package main + +type S struct { + foo string +} + +func main() { + var s S = S{foo: "bar"} + + print(s.foo) +} \ No newline at end of file diff --git a/testData/intentions/move-to-struct-initialization/structDeclarationWithEmptyInit.go b/testData/intentions/move-to-struct-initialization/structDeclarationWithEmptyInit.go new file mode 100644 index 0000000000..faea7a738a --- /dev/null +++ b/testData/intentions/move-to-struct-initialization/structDeclarationWithEmptyInit.go @@ -0,0 +1,11 @@ +package main + +type S struct { + foo string +} + +func main() { + var s S = S{} + s.foo = "bar" + print(s.foo) +} \ No newline at end of file diff --git a/testData/intentions/move-to-struct-initialization/structDeclarationWithNonEmptyInit-after.go b/testData/intentions/move-to-struct-initialization/structDeclarationWithNonEmptyInit-after.go new file mode 100644 index 0000000000..7da47a03be --- /dev/null +++ b/testData/intentions/move-to-struct-initialization/structDeclarationWithNonEmptyInit-after.go @@ -0,0 +1,12 @@ +package main + +type S struct { + foo string + bar string +} + +func main() { + var s S = S{bar: "a", foo: "bar"} + + print(s.foo) +} \ No newline at end of file diff --git a/testData/intentions/move-to-struct-initialization/structDeclarationWithNonEmptyInit.go b/testData/intentions/move-to-struct-initialization/structDeclarationWithNonEmptyInit.go new file mode 100644 index 0000000000..64441d8677 --- /dev/null +++ b/testData/intentions/move-to-struct-initialization/structDeclarationWithNonEmptyInit.go @@ -0,0 +1,12 @@ +package main + +type S struct { + foo string + bar string +} + +func main() { + var s S = S{bar: "a"} + s.foo = "bar" + print(s.foo) +} \ No newline at end of file diff --git a/testData/intentions/move-to-struct-initialization/structDeclarationWrongQualifier.go b/testData/intentions/move-to-struct-initialization/structDeclarationWrongQualifier.go new file mode 100644 index 0000000000..ef50a9c4ec --- /dev/null +++ b/testData/intentions/move-to-struct-initialization/structDeclarationWrongQualifier.go @@ -0,0 +1,12 @@ +package main + +type S struct { + foo string +} + +func main() { + var s S + var b S + s.foo = "bar" + print(b.foo) +} \ No newline at end of file diff --git a/tests/com/goide/quickfix/GoMoveToStructInitializationIntentionTest.java b/tests/com/goide/quickfix/GoMoveToStructInitializationIntentionTest.java index 84fe19eda1..3c7a0123c6 100644 --- a/tests/com/goide/quickfix/GoMoveToStructInitializationIntentionTest.java +++ b/tests/com/goide/quickfix/GoMoveToStructInitializationIntentionTest.java @@ -50,10 +50,15 @@ protected String getBasePath() { public void testMultipleFieldsPartlyAssigned() { doTest(); } public void testWithParens() { doTest(); } public void testFieldExtractedFromParens() { doTest(); } + public void testStructDeclaration() { doTest(); } + public void testParensStructDeclaration() { doTest(); } + public void testStructDeclarationWithEmptyInit() { doTest(); } + public void testStructDeclarationWithNonEmptyInit() { doTest(); } public void testDuplicateFields() { doTest(); } public void testMultiReturnFunction() { doTestNoFix(); } public void testWrongStruct() { doTestNoFix(); } + public void testNotStructDeclaration() { doTestNoFix(); } public void testExistingDeclaration() { doTestNoFix(); } public void testNotExistingField() { doTestNoFix(); } public void testJustAssignedVarWrongCaret() { doTestNoFix(); } @@ -61,5 +66,7 @@ protected String getBasePath() { public void testJustInitializedVarWrongCaret() { doTestNoFix(); } public void testJustAssignedVarBothParens() { doTestNoFix(); } public void testJustAssignedFieldParens() { doTestNoFix(); } + public void testStructDeclarationMultipleExpressions() { doTestNoFix(); } + public void testStructDeclarationWrongQualifier() { doTestNoFix(); } } From b0d7f3beeb1e643d3d61fc4f89f10c0ad189541b Mon Sep 17 00:00:00 2001 From: Sergey Ibragimov Date: Tue, 8 Nov 2016 10:46:11 +0300 Subject: [PATCH 2/3] rename remove getters for DTO fix notnull and nullable annotations --- ...GoMoveToStructInitializationIntention.java | 169 +++++++----------- 1 file changed, 66 insertions(+), 103 deletions(-) diff --git a/src/com/goide/intentions/GoMoveToStructInitializationIntention.java b/src/com/goide/intentions/GoMoveToStructInitializationIntention.java index b32aaf145f..9290b2adb8 100644 --- a/src/com/goide/intentions/GoMoveToStructInitializationIntention.java +++ b/src/com/goide/intentions/GoMoveToStructInitializationIntention.java @@ -60,20 +60,19 @@ public boolean isAvailable(@NotNull Project project, Editor editor, @NotNull Psi private static Data getData(@NotNull PsiElement element) { if (!element.isValid() || !element.isWritable()) return null; GoAssignmentStatement assignment = getValidAssignmentParent(element); - GoStatement previousStatement = assignment != null ? PsiTreeUtil.getPrevSiblingOfType(assignment, GoStatement.class) : null; - GoReferenceExpression selectedFieldReferenceExpression = - previousStatement != null ? getFieldReferenceExpression(element, assignment, previousStatement) : null; - if (selectedFieldReferenceExpression == null) return null; + GoStatement prevStatement = assignment != null ? PsiTreeUtil.getPrevSiblingOfType(assignment, GoStatement.class) : null; + GoReferenceExpression selectedExpression = prevStatement != null + ? getFieldReferenceExpression(element, assignment, prevStatement) : null; + if (selectedExpression == null) return null; - GoVarDefinition structDefinition = getDefinition(selectedFieldReferenceExpression); - boolean needReplaceDeclarationWithShortVar = isUnassigned(getSingleVarSpecByDefinition(previousStatement, structDefinition)); + GoVarDefinition definition = getDefinition(selectedExpression); + boolean needReplaceDeclaration = isUnassigned(getSingleVarSpec(prevStatement, definition)); - GoCompositeLit compositeLit = getStructLiteralByReference(selectedFieldReferenceExpression, previousStatement); - if (compositeLit == null && !needReplaceDeclarationWithShortVar || structDefinition == null) return null; + GoCompositeLit compositeLit = getStructLiteralByReference(selectedExpression, prevStatement); + if (compositeLit == null && !needReplaceDeclaration || definition == null) return null; - List references = - getUninitializedSingleFieldReferences(assignment, previousStatement, structDefinition, compositeLit); - return !references.isEmpty() ? new Data(assignment, compositeLit, references, previousStatement, structDefinition) : null; + List expressions = getUninitializedReferenceExpressions(assignment, prevStatement, definition, compositeLit); + return !expressions.isEmpty() ? new Data(assignment, compositeLit, expressions, prevStatement, definition) : null; } @Nullable @@ -83,8 +82,7 @@ private static GoCompositeLit createStructLiteral(@NotNull GoVarDefinition defin } @Nullable - @Contract("null -> null") - private static GoVarDefinition getDefinition(@Nullable GoReferenceExpression referenceExpressions) { + private static GoVarDefinition getDefinition(@NotNull GoReferenceExpression referenceExpressions) { return ObjectUtils.tryCast(resolveQualifier(referenceExpressions), GoVarDefinition.class); } @@ -98,16 +96,15 @@ private static GoAssignmentStatement getValidAssignmentParent(@Nullable PsiEleme @Nullable private static GoReferenceExpression getFieldReferenceExpression(@NotNull PsiElement selectedElement, @NotNull GoAssignmentStatement assignment, - @NotNull GoStatement previousStatement) { - GoReferenceExpression selectedReferenceExpression = PsiTreeUtil.getTopmostParentOfType(selectedElement, GoReferenceExpression.class); - if (isFieldReferenceExpression(selectedReferenceExpression)) { - return !isAssignedInStatement(getRightExpression(selectedReferenceExpression, assignment), previousStatement) - ? selectedReferenceExpression : null; + @NotNull GoStatement prevStatement) { + GoReferenceExpression selectedExpression = PsiTreeUtil.getTopmostParentOfType(selectedElement, GoReferenceExpression.class); + if (isFieldReferenceExpression(selectedExpression)) { + return !isAssignedInStatement(getRightExpression(selectedExpression, assignment), prevStatement) ? selectedExpression : null; } List fieldReferenceExpressions = getFieldReferenceExpressions(assignment); - if (exists(fieldReferenceExpressions, - expression -> isAssignedInStatement(getRightExpression(expression, assignment), previousStatement))) { + if (exists(fieldReferenceExpressions, expression -> + isAssignedInStatement(getRightExpression(expression, assignment), prevStatement))) { return null; } @@ -131,8 +128,7 @@ private static GoReferenceExpression unwrapParensAndCast(@Nullable PsiElement el @Nullable @Contract("_, null -> null; null, _ -> null") - private static GoVarSpec getSingleVarSpecByDefinition(@Nullable GoStatement statement, - @Nullable GoVarDefinition definition) { + private static GoVarSpec getSingleVarSpec(@Nullable GoStatement statement, @Nullable GoVarDefinition definition) { GoVarDeclaration declaration = statement != null ? statement.getVarDeclaration() : null; List varSpecs = declaration != null ? declaration.getVarSpecList() : emptyList(); GoVarSpec singleVarSpec = varSpecs.size() == 1 ? getFirstItem(varSpecs) : null; @@ -146,8 +142,8 @@ private static boolean isUnassigned(@Nullable GoVarSpec varSpec) { } @Contract("null -> false") - private static boolean isFieldReferenceExpression(@Nullable PsiElement element) { - return element instanceof GoReferenceExpression && isFieldDefinition(((GoReferenceExpression)element).resolve()); + private static boolean isFieldReferenceExpression(@Nullable GoReferenceExpression element) { + return element != null && isFieldDefinition(element.resolve()); } @Contract("null -> false") @@ -155,15 +151,13 @@ private static boolean isFieldDefinition(@Nullable PsiElement element) { return element instanceof GoFieldDefinition || element instanceof GoAnonymousFieldDefinition; } - private static boolean isAssignedInStatement(@Nullable GoReferenceExpression referenceExpression, - @NotNull GoStatement statement) { + private static boolean isAssignedInStatement(@Nullable GoReferenceExpression referenceExpression, @NotNull GoStatement statement) { PsiElement resolve = referenceExpression != null ? referenceExpression.resolve() : null; - return exists(getLeftHandElements(statement), element -> isResolvedTo(element, resolve)); + return resolve != null && exists(getLeftHandElements(statement), element -> isResolvedTo(element, resolve)); } @Nullable - private static GoReferenceExpression getRightExpression(@NotNull GoExpression expression, - @NotNull GoAssignmentStatement assignment) { + private static GoReferenceExpression getRightExpression(@NotNull GoExpression expression, @NotNull GoAssignmentStatement assignment) { return unwrapParensAndCast(GoPsiImplUtil.getRightExpression(assignment, getTopmostExpression(expression))); } @@ -172,7 +166,7 @@ private static GoExpression getTopmostExpression(@NotNull GoExpression expressio return ObjectUtils.notNull(PsiTreeUtil.getTopmostParentOfType(expression, GoExpression.class), expression); } - private static boolean isResolvedTo(@Nullable PsiElement element, @Nullable PsiElement resolve) { + private static boolean isResolvedTo(@Nullable PsiElement element, @NotNull PsiElement resolve) { if (element instanceof GoVarDefinition) return resolve == element; GoReferenceExpression refExpression = unwrapParensAndCast(element); @@ -180,14 +174,14 @@ private static boolean isResolvedTo(@Nullable PsiElement element, @Nullable PsiE } @NotNull - private static List getUninitializedSingleFieldReferences(@NotNull GoAssignmentStatement assignment, - @NotNull GoStatement previousStatement, - @Nullable GoVarDefinition definition, - @Nullable GoCompositeLit compositeLit) { - List uninitializedFieldReferencesByQualifier = - filter(getUninitializedFieldReferenceExpressions(assignment, compositeLit, previousStatement), - element -> isResolvedTo(element.getQualifier(), definition)); - MultiMap resolved = groupBy(uninitializedFieldReferencesByQualifier, GoReferenceExpression::resolve); + private static List getUninitializedReferenceExpressions(@NotNull GoAssignmentStatement assignment, + @NotNull GoStatement prevStatement, + @NotNull GoVarDefinition definition, + @Nullable GoCompositeLit compositeLit) { + List uninitializedFieldReferences = filter( + getUninitializedFieldReferenceExpressions(assignment, compositeLit, prevStatement), + element -> isResolvedTo(element.getQualifier(), definition)); + MultiMap resolved = groupBy(uninitializedFieldReferences, GoReferenceExpression::resolve); return map(filter(resolved.entrySet(), set -> set.getValue().size() == 1), set -> getFirstItem(set.getValue())); } @@ -216,8 +210,8 @@ private static GoCompositeLit getStructLiteral(@NotNull GoReferenceExpression fi @Nullable @Contract("null -> null") - private static PsiElement resolveQualifier(@Nullable GoReferenceExpression fieldReferenceExpression) { - GoReferenceExpression qualifier = fieldReferenceExpression != null ? fieldReferenceExpression.getQualifier() : null; + private static PsiElement resolveQualifier(@NotNull GoReferenceExpression fieldReferenceExpression) { + GoReferenceExpression qualifier = fieldReferenceExpression.getQualifier(); return qualifier != null ? qualifier.resolve() : null; } @@ -240,7 +234,7 @@ private static GoCompositeLit getStructLiteral(@NotNull GoReferenceExpression fi @Nullable @Contract("null, _ -> null") private static GoCompositeLit getStructLiteral(@Nullable GoVarDefinition definition, @NotNull GoStatement statement) { - GoVarSpec varSpec = definition != null ? getSingleVarSpecByDefinition(statement, definition) : null; + GoVarSpec varSpec = definition != null ? getSingleVarSpec(statement, definition) : null; return varSpec != null ? ObjectUtils.tryCast(getFirstItem(varSpec.getRightExpressionsList()), GoCompositeLit.class) : null; } @@ -258,23 +252,20 @@ private static boolean isFieldInitialization(@NotNull GoElement element, @NotNul @NotNull private static List getUninitializedFieldReferenceExpressions(@NotNull GoAssignmentStatement assignment, - @Nullable GoCompositeLit structLiteral, - @NotNull GoStatement previousStatement) { - return filter(getFieldReferenceExpressions(assignment), - expression -> isUninitializedFieldReferenceExpression(expression, structLiteral) && - !isAssignedInStatement(getRightExpression(expression, assignment), previousStatement)); + @Nullable GoCompositeLit compositeLit, + @NotNull GoStatement prevStatement) { + return filter(getFieldReferenceExpressions(assignment), expression -> + isUninitializedFieldReferenceExpression(expression, compositeLit) && + !isAssignedInStatement(getRightExpression(expression, assignment), prevStatement)); } - @Contract("null, _-> false; !null, null -> true") - private static boolean isUninitializedFieldReferenceExpression(@Nullable GoReferenceExpression fieldReferenceExpression, - @Nullable GoCompositeLit structLiteral) { - if (fieldReferenceExpression == null) return false; - if (structLiteral == null) return true; - - GoLiteralValue literalValue = structLiteral.getLiteralValue(); - PsiElement resolve = fieldReferenceExpression.resolve(); - return literalValue != null && isFieldDefinition(resolve) && - !exists(literalValue.getElementList(), element -> isFieldInitialization(element, resolve)); + @Contract("_, null -> true") + private static boolean isUninitializedFieldReferenceExpression(@NotNull GoReferenceExpression fieldReferenceExpression, + @Nullable GoCompositeLit compositeLit) { + if (compositeLit == null) return true; + GoLiteralValue literalValue = compositeLit.getLiteralValue(); + PsiElement resolve = literalValue != null ? fieldReferenceExpression.resolve() : null; + return isFieldDefinition(resolve) && !exists(literalValue.getElementList(), element -> isFieldInitialization(element, resolve)); } @NotNull @@ -294,16 +285,16 @@ public void invoke(@NotNull Project project, Editor editor, @NotNull PsiElement Data data = getData(element); if (data == null) return; - boolean needReplaceDeclarationWithShortVar = data.getCompositeLit() == null; - GoCompositeLit compositeLit = - needReplaceDeclarationWithShortVar ? createStructLiteral(data.getStructDefinition(), project) : data.getCompositeLit(); - if (compositeLit == null || needReplaceDeclarationWithShortVar && data.getStructDeclaration() == null) return; + boolean needReplaceDeclaration = data.myCompositeLit == null; + GoCompositeLit compositeLit = needReplaceDeclaration ? createStructLiteral(data.myStructDefinition, project) : data.myCompositeLit; + if (compositeLit == null) return; - moveFieldReferenceExpressions(data.getReferenceExpressions(), compositeLit.getLiteralValue(), data.getAssignment()); - if (!needReplaceDeclarationWithShortVar) return; - GoStatement shortVarStatement = - GoElementFactory.createShortVarDeclarationStatement(project, data.getStructDefinition().getText(), compositeLit.getText()); - data.getStructDeclaration().replace(shortVarStatement); + moveFieldReferenceExpressions(data.myReferenceExpressions, compositeLit.getLiteralValue(), data.myAssignment); + if (needReplaceDeclaration) { + String definitionText = data.myStructDefinition.getText(); + GoStatement shortVarStatement = GoElementFactory.createShortVarDeclarationStatement(project, definitionText, compositeLit.getText()); + data.myStructDeclaration.replace(shortVarStatement); + } } private static void moveFieldReferenceExpressions(@NotNull List referenceExpressions, @@ -323,27 +314,24 @@ private static void moveFieldReferenceExpressions(@NotNull List myReferenceExpressions; - private final GoStatement myStructDeclaration; - private final GoVarDefinition myStructDefinition; + @Nullable private final GoCompositeLit myCompositeLit; + @NotNull private final GoAssignmentStatement myAssignment; + @NotNull private final List myReferenceExpressions; + @NotNull private final GoStatement myStructDeclaration; + @NotNull private final GoVarDefinition myStructDefinition; public Data(@NotNull GoAssignmentStatement assignment, @Nullable GoCompositeLit compositeLit, @NotNull List referenceExpressions, - @Nullable GoStatement structDeclaration, + @NotNull GoStatement structDeclaration, @NotNull GoVarDefinition structDefinition) { myCompositeLit = compositeLit; myAssignment = assignment; @@ -351,31 +339,6 @@ public Data(@NotNull GoAssignmentStatement assignment, myStructDeclaration = structDeclaration; myStructDefinition = structDefinition; } - - @Nullable - public GoCompositeLit getCompositeLit() { - return myCompositeLit; - } - - @NotNull - public GoAssignmentStatement getAssignment() { - return myAssignment; - } - - @NotNull - public List getReferenceExpressions() { - return myReferenceExpressions; - } - - @Nullable - public GoStatement getStructDeclaration() { - return myStructDeclaration; - } - - @NotNull - public GoVarDefinition getStructDefinition() { - return myStructDefinition; - } } } From a267834be40f2a0a8d66c28edc0a59836631872e Mon Sep 17 00:00:00 2001 From: Sergey Ibragimov Date: Tue, 8 Nov 2016 11:09:42 +0300 Subject: [PATCH 3/3] dont invoke intention for field of anon field --- ...GoMoveToStructInitializationIntention.java | 35 ++++++++++++------- .../fieldOfAnonymousField.go | 15 ++++++++ ...veToStructInitializationIntentionTest.java | 1 + 3 files changed, 38 insertions(+), 13 deletions(-) create mode 100644 testData/intentions/move-to-struct-initialization/fieldOfAnonymousField.go diff --git a/src/com/goide/intentions/GoMoveToStructInitializationIntention.java b/src/com/goide/intentions/GoMoveToStructInitializationIntention.java index 9290b2adb8..9cc7789727 100644 --- a/src/com/goide/intentions/GoMoveToStructInitializationIntention.java +++ b/src/com/goide/intentions/GoMoveToStructInitializationIntention.java @@ -66,12 +66,16 @@ private static Data getData(@NotNull PsiElement element) { if (selectedExpression == null) return null; GoVarDefinition definition = getDefinition(selectedExpression); - boolean needReplaceDeclaration = isUnassigned(getSingleVarSpec(prevStatement, definition)); + GoType type = definition != null ? definition.getGoType(null) : null; + GoStructType structType = type != null ? ObjectUtils.tryCast(type.getUnderlyingType(), GoStructType.class) : null; + if (structType == null) return null; - GoCompositeLit compositeLit = getStructLiteralByReference(selectedExpression, prevStatement); - if (compositeLit == null && !needReplaceDeclaration || definition == null) return null; + boolean needReplaceDeclaration = isUnassigned(getSingleVarSpec(prevStatement, definition)); + GoCompositeLit compositeLit = !needReplaceDeclaration ? getStructLiteralByReference(selectedExpression, prevStatement) : null; + if (compositeLit == null && !needReplaceDeclaration) return null; - List expressions = getUninitializedReferenceExpressions(assignment, prevStatement, definition, compositeLit); + List expressions = getUninitializedReferenceExpressions(assignment, prevStatement, definition, compositeLit, + structType); return !expressions.isEmpty() ? new Data(assignment, compositeLit, expressions, prevStatement, definition) : null; } @@ -177,9 +181,10 @@ private static boolean isResolvedTo(@Nullable PsiElement element, @NotNull PsiEl private static List getUninitializedReferenceExpressions(@NotNull GoAssignmentStatement assignment, @NotNull GoStatement prevStatement, @NotNull GoVarDefinition definition, - @Nullable GoCompositeLit compositeLit) { + @Nullable GoCompositeLit compositeLit, + @NotNull GoStructType type) { List uninitializedFieldReferences = filter( - getUninitializedFieldReferenceExpressions(assignment, compositeLit, prevStatement), + getUninitializedFieldReferenceExpressions(assignment, prevStatement, compositeLit, type), element -> isResolvedTo(element.getQualifier(), definition)); MultiMap resolved = groupBy(uninitializedFieldReferences, GoReferenceExpression::resolve); return map(filter(resolved.entrySet(), set -> set.getValue().size() == 1), set -> getFirstItem(set.getValue())); @@ -252,19 +257,23 @@ private static boolean isFieldInitialization(@NotNull GoElement element, @NotNul @NotNull private static List getUninitializedFieldReferenceExpressions(@NotNull GoAssignmentStatement assignment, + @NotNull GoStatement prevStatement, @Nullable GoCompositeLit compositeLit, - @NotNull GoStatement prevStatement) { + @NotNull GoStructType type) { return filter(getFieldReferenceExpressions(assignment), expression -> - isUninitializedFieldReferenceExpression(expression, compositeLit) && + isUninitializedFieldReferenceExpression(expression, compositeLit, type) && !isAssignedInStatement(getRightExpression(expression, assignment), prevStatement)); } - @Contract("_, null -> true") private static boolean isUninitializedFieldReferenceExpression(@NotNull GoReferenceExpression fieldReferenceExpression, - @Nullable GoCompositeLit compositeLit) { - if (compositeLit == null) return true; - GoLiteralValue literalValue = compositeLit.getLiteralValue(); - PsiElement resolve = literalValue != null ? fieldReferenceExpression.resolve() : null; + @Nullable GoCompositeLit compositeLit, + @NotNull GoStructType type) { + PsiElement resolve = fieldReferenceExpression.resolve(); + if (resolve == null || !PsiTreeUtil.isAncestor(type, resolve, true)) return false; + + GoLiteralValue literalValue = compositeLit != null ? compositeLit.getLiteralValue() : null; + if (literalValue == null) return true; + return isFieldDefinition(resolve) && !exists(literalValue.getElementList(), element -> isFieldInitialization(element, resolve)); } diff --git a/testData/intentions/move-to-struct-initialization/fieldOfAnonymousField.go b/testData/intentions/move-to-struct-initialization/fieldOfAnonymousField.go new file mode 100644 index 0000000000..2b0fe6fa3f --- /dev/null +++ b/testData/intentions/move-to-struct-initialization/fieldOfAnonymousField.go @@ -0,0 +1,15 @@ +package main + +type S struct { + B +} + +type B struct { + bar string +} + +func main() { + var s S + s.bar = "bar" + print(s.bar) +} \ No newline at end of file diff --git a/tests/com/goide/quickfix/GoMoveToStructInitializationIntentionTest.java b/tests/com/goide/quickfix/GoMoveToStructInitializationIntentionTest.java index 3c7a0123c6..e8750cdf0b 100644 --- a/tests/com/goide/quickfix/GoMoveToStructInitializationIntentionTest.java +++ b/tests/com/goide/quickfix/GoMoveToStructInitializationIntentionTest.java @@ -68,5 +68,6 @@ protected String getBasePath() { public void testJustAssignedFieldParens() { doTestNoFix(); } public void testStructDeclarationMultipleExpressions() { doTestNoFix(); } public void testStructDeclarationWrongQualifier() { doTestNoFix(); } + public void testFieldOfAnonymousField() { doTestNoFix(); } }