Move to struct init intention: handle struct declaration case #2816

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Closed

wbars wants to merge 1 commit into go-lang-plugin-org:master from wbars:move-to-struct

Contributor

wbars commented Nov 1, 2016

Replace single struct declaration with short var declaration, or add to existing struct literal in statement.

wbars changed the title ~~Move to struct declaration: handle struct declaration case~~ Move to struct init intention: handle struct declaration case

grenki reviewed

View reviewed changes

src/com/goide/psi/impl/GoElementFactory.java

+                @NotNull
+                public static GoCompositeLit createCompositeLit(@NotNull Project project, @NotNull GoType type) {
+                  String typeText = type.getText();
+                  GoFile file = createFileFromText(project, "package a; struct " + typeText + " {}; var _ = " + typeText + "{}");

Contributor

grenki Nov 1, 2016

Why you need this : struct " + typeText + " {}; ?

src/com/goide/intentions/GoMoveToStructInitializationIntention.java

                   GoStatement previousStatement = PsiTreeUtil.getPrevSiblingOfType(assignment, GoStatement.class);
                   if (previousStatement instanceof GoSimpleStatement) {
                     return getStructLiteral(fieldReferenceExpression, (GoSimpleStatement)previousStatement);
                   }
                   if (previousStatement instanceof GoAssignmentStatement) {
                     return getStructLiteral(fieldReferenceExpression, (GoAssignmentStatement)previousStatement);
                   }
+                  if (previousStatement instanceof GoStatementImpl) {

Contributor

grenki Nov 1, 2016

Use GoStatement interface instead GoStatementImpl

src/com/goide/intentions/GoMoveToStructInitializationIntention.java

+                  if (varSpec == null) return null;
+                  GoType structType = isUnassigned(varSpec) ? definition.getGoType(null) : null;
+                  return structType != null
+                         ? GoElementFactory.createCompositeLit(project, structType)

Contributor

grenki Nov 1, 2016

You can take project from statement

src/com/goide/intentions/GoMoveToStructInitializationIntention.java

+                  if (varSpec == null) return null;
+                  GoType structType = isUnassigned(varSpec) ? definition.getGoType(null) : null;
+                  return structType != null
+                         ? GoElementFactory.createCompositeLit(project, structType)

Contributor

grenki Nov 1, 2016

You can take project from statement

src/com/goide/intentions/GoMoveToStructInitializationIntention.java

+                  List<GoVarSpec> varSpecs = declaration != null ? declaration.getVarSpecList() : emptyList();
+                  GoVarSpec singleVarSpec = varSpecs.size() == 1 ? getFirstItem(varSpecs) : null;
+                  List<GoVarDefinition> varDefinitions = singleVarSpec != null ? singleVarSpec.getVarDefinitionList() : emptyList();
+                  return varDefinitions.size() == 1 && isResolvedTo(definition, getFirstItem(varDefinitions)) ? singleVarSpec : null;

Contributor

grenki Nov 1, 2016

If you know that definition is GoVarDefinition use == or equals

src/com/goide/intentions/GoMoveToStructInitializationIntention.java

                   return ObjectUtils.tryCast(compositeLit, GoCompositeLit.class);
                 }
+                @Nullable
+                private static GoCompositeLit getStructLiteral(@NotNull GoStatementImpl statement,

Contributor

grenki Nov 1, 2016

It's getOrCreateStructLiteral and better create composite literal more clear.
you check was struct literal created again in getData : isUnassigned(getSingleVarSpecByDefinition(previousStatement, structDefinition)))

wbars force-pushed the move-to-struct branch from fc9fd3f to 1be6770 Compare

November 2, 2016 10:37

Contributor Author

wbars commented Nov 3, 2016

Force pushed

grenki reviewed

View reviewed changes

src/com/goide/intentions/GoMoveToStructInitializationIntention.java

+                  if (!data.needReplaceDeclarationWithShortVar()) return;
+                  data.getStructDeclaration()
+                    .replace(GoElementFactory.createShortVarDeclarationStatement(project, data.getStructVarName(), data.getCompositeLit().getText()));

Contributor

grenki Nov 3, 2016

extract variable for more readability, please

src/com/goide/intentions/GoMoveToStructInitializationIntention.java

+                  boolean needReplaceDeclarationWithShortVar = isUnassigned(getSingleVarSpecByDefinition(previousStatement, structDefinition));
+                  GoCompositeLit compositeLit = needReplaceDeclarationWithShortVar
+                                                ? createStructLiteral(structDefinition, previousStatement.getProject())

Contributor

grenki Nov 3, 2016

You shouldn't create struct literal at this place. Because method name is getData and it's called in isAviable

src/com/goide/intentions/GoMoveToStructInitializationIntention.java

                   Data data = getData(element);
                   if (data == null) return;
                   moveFieldReferenceExpressions(data);
+                  if (!data.needReplaceDeclarationWithShortVar()) return;
+                  data.getStructDeclaration()

Contributor

grenki Nov 3, 2016

NPE can be there

src/com/goide/intentions/GoMoveToStructInitializationIntention.java

@@ @@ -292,5 +352,19 @@ public GoAssignmentStatement getAssignment() { @@
                   public List<GoReferenceExpression> getReferenceExpressions() {
                     return myReferenceExpressions;
                   }
+                  public GoStatement getStructDeclaration() {

Contributor

grenki Nov 3, 2016

annotate with @NotNull all getters please

src/com/goide/intentions/GoMoveToStructInitializationIntention.java

+                              @NotNull List<GoReferenceExpression> referenceExpressions,
+                              @Nullable GoStatement structDeclaration,
+                              @Nullable String structVarName,
+                              boolean needReplaceDeclarationWithShortVar) {

Contributor

grenki Nov 3, 2016

I think you can get rid of needReplaceDeclarationWithShortVar field by giving null compositeLit

wbars force-pushed the move-to-struct branch 2 times, most recently from f8a68be to fdb56e7 Compare

November 4, 2016 07:32

Contributor Author

wbars commented Nov 4, 2016

Updated

wbars force-pushed the move-to-struct branch 4 times, most recently from 57a8f43 to 7640c4f Compare

November 6, 2016 09:56


          Move to struct declaration: handle struct declaration via _var_ case

c9140d5

Replace single struct declaration with short var declaration, or add to existing struct literal in statement

wbars force-pushed the move-to-struct branch from 7640c4f to c9140d5 Compare

November 7, 2016 20:03

Contributor

grenki commented Nov 8, 2016 •

edited

Loading

@wbars Thank you for this work.

I make few changes as renaming and fix notnull annotations, fix case for field of anonymous field and create another pull reqest. You can see changes here: #2822

Contributor Author

wbars commented Nov 8, 2016

@grenki Still can't fully beat naming problem, I guess :) Thanks for the review.

wbars closed this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet