Skip to content
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.

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

Improve move to struct initialization inspection. Fix pull request from wbars #2822

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

grenki
Copy link
Contributor

@grenki grenki commented Nov 8, 2016

No description provided.

wbars and others added 3 commits November 6, 2016 19:56
Replace single struct declaration with short var declaration, or add to existing struct literal in statement
remove getters for DTO
fix notnull and nullable annotations
@wbars
Copy link
Contributor

wbars commented Nov 8, 2016

This PR created from an old commit :( Last commit

@zolotov
Copy link
Contributor

zolotov commented Nov 14, 2016

What the state of the PR?
Who is responsible for tests failing?

cc: @grenki

@grenki
Copy link
Contributor Author

grenki commented Nov 18, 2016

@zolotov PR ready merge. Tests are ok

@zolotov zolotov self-assigned this Nov 18, 2016
Copy link
Contributor

@zolotov zolotov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems unsupportable. refactoring is required

PsiElement resolve = fieldReferenceExpression.resolve();
return literalValue != null && isFieldDefinition(resolve) &&
!exists(literalValue.getElementList(), element -> isFieldInitialization(element, resolve));
if (resolve == null || !PsiTreeUtil.isAncestor(type, resolve, true)) return false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's unclear why if the field is a descendant of some type then the field reference is uninitialized. Where is the link?

GoLiteralValue literalValue = data.getCompositeLit().getLiteralValue();
private static void moveFieldReferenceExpressions(@NotNull List<GoReferenceExpression> referenceExpressions,
@Nullable GoLiteralValue literalValue,
@NotNull GoAssignmentStatement parentAssignment) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the difference between assigment parameter which is used in almost all methods and parentAssignment

}

private static void moveFieldReferenceExpressions(@NotNull Data data) {
GoLiteralValue literalValue = data.getCompositeLit().getLiteralValue();
private static void moveFieldReferenceExpressions(@NotNull List<GoReferenceExpression> referenceExpressions,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's happened with DTO here? Any reason to pass 3 parameters here?

@@ -60,12 +60,34 @@ public boolean isAvailable(@NotNull Project project, Editor editor, @NotNull Psi
private static Data getData(@NotNull PsiElement element) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getData method became overcomplicated. In my opinion, such methods should look like this:

val data
data.somePieceOfData1 = gatherSomePieceOfData_1()
data.somePieceOfData2 = gatherSomePieceOfData_2()
data.somePieceOfData3 = gatherSomePieceOfData_3()
return data

For now it looks like endless if, if, != null, return null. It's barely readable and hard to edit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants