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.

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

INT & ANN: Implement intention and fix for struct fields expansion instead of .. #3996

Merged
merged 1 commit into from Aug 19, 2019
Merged

INT & ANN: Implement intention and fix for struct fields expansion instead of .. #3996

merged 1 commit into from Aug 19, 2019

Conversation

shevtsiv
Copy link
Contributor

@shevtsiv shevtsiv commented Jun 9, 2019 •
edited by Undin

In this pull request I intend to do the following:

  1. Create E0023 and E0027 error classes and implement analysis for them.
  2. Create fix for those errors.
  3. Create intention for struct fields expansion.
  4. Create a module with shared code for using it in the fix and intention.

Once these steps are complete I will squash all the commits and #3928 should be closed after merge.
2019-08-26 13 18 04
2019-08-24 19 22 36

@shevtsiv
Copy link
Contributor Author

shevtsiv commented Jun 9, 2019

Although this is a draft proposals and ideas are welcome.

@shevtsiv shevtsiv force-pushed the expand_fields_intention branch 2 times, most recently from 7e676ed to b5bc990 Compare June 28, 2019 14:37
@shevtsiv shevtsiv changed the title [INT & ANN] Implement intention and fix for struct fields expansion instead of .. INT & ANN: Implement intention and fix for struct fields expansion instead of .. Jun 28, 2019
@Undin Undin added the feature label Jul 4, 2019
@shevtsiv shevtsiv force-pushed the expand_fields_intention branch 2 times, most recently from a6cc04a to 72371fe Compare July 7, 2019 09:03
@shevtsiv shevtsiv marked this pull request as ready for review July 8, 2019 10:57
@shevtsiv shevtsiv force-pushed the expand_fields_intention branch 2 times, most recently from 72ee5e2 to 7278d7f Compare July 18, 2019 10:22
@mchernyavsky mchernyavsky added this to the v102 milestone Jul 18, 2019
@mchernyavsky mchernyavsky removed this from the v102 milestone Jul 18, 2019
@shevtsiv shevtsiv force-pushed the expand_fields_intention branch 5 times, most recently from 71cb5e5 to b13c2c7 Compare July 22, 2019 18:46
vlad20012
vlad20012 reviewed Aug 5, 2019
View reviewed changes
Copy link
Member

@vlad20012 vlad20012 left a comment

Choose a reason for hiding this comment

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

Also take into account the case let Foo(a, .., b) in the intention tests, looks like now it throws an exception

src/main/kotlin/org/rust/ide/annotator/fixes/ExpandStructFieldsFix.kt Outdated Show resolved Hide resolved
src/main/kotlin/org/rust/ide/annotator/RsErrorAnnotator.kt Show resolved Hide resolved
src/main/kotlin/org/rust/ide/utils/StructFieldsExpander.kt Outdated Show resolved Hide resolved
src/main/kotlin/org/rust/ide/utils/StructFieldsExpander.kt Outdated Show resolved Hide resolved
src/main/kotlin/org/rust/ide/intentions/ExpandStructFieldsIntention.kt Outdated Show resolved Hide resolved
src/main/kotlin/org/rust/ide/utils/StructFieldsExpander.kt Outdated Show resolved Hide resolved
src/main/kotlin/org/rust/ide/utils/StructFieldsExpander.kt Outdated Show resolved Hide resolved
src/main/kotlin/org/rust/ide/utils/StructFieldsExpander.kt Outdated Show resolved Hide resolved
src/main/kotlin/org/rust/ide/utils/StructFieldsExpander.kt Outdated Show resolved Hide resolved
@shevtsiv shevtsiv force-pushed the expand_fields_intention branch 5 times, most recently from c8ab108 to ef5e5a9 Compare August 12, 2019 16:20
@shevtsiv shevtsiv force-pushed the expand_fields_intention branch 7 times, most recently from d1efbca to 0316e0e Compare August 19, 2019 15:17
@vlad20012
Copy link
Member

bors r+

bors bot added a commit that referenced this pull request Aug 19, 2019
@bors @shevtsiv @vlad20012
Merge #3996 #4094
472af45 
3996: INT & ANN: Implement intention and fix for struct fields expansion instead of `..` r=vlad20012 a=shevtsiv

<!--
Hello and thank you for the pull request!

We don't have any strict rules about pull requests, but you might check
https://github.com/intellij-rust/intellij-rust/blob/master/CONTRIBUTING.md
for some hints!

Note that we need an electronic CLA for contributions:
https://github.com/intellij-rust/intellij-rust/blob/master/CONTRIBUTING.md#cla

After you sign the CLA, please add your name to
https://github.com/intellij-rust/intellij-rust/blob/master/CONTRIBUTORS.txt

:)
-->
In this pull request I intend to do the following:
1) Create E0023 and E0027 error classes and implement analysis for them.
2) Create fix for those errors.
3) Create intention for struct fields expansion.
4) Create a module with shared code for using it in the fix and intention.

Once these steps are complete I will squash all the commits and #3928 should be closed after merge.

4094: TY&RES: implement hygiene & infer types in macros r=undin a=vlad20012

Now we take into account any custom macros in type inference

Co-authored-by: shevtsiv <rostykshevtsiv@gmail.com>
Co-authored-by: vlad20012 <beskvlad@gmail.com>
@vlad20012 vlad20012 added this to In Progress in To test via automation Aug 19, 2019
@vlad20012 vlad20012 added this to the v104 milestone Aug 19, 2019
@vlad20012 vlad20012 moved this from In Progress to Test in To test Aug 19, 2019
@bors
Copy link
Contributor

bors bot commented Aug 19, 2019

Build failed (retrying...)

bors bot added a commit that referenced this pull request Aug 19, 2019
@bors @shevtsiv
Merge #3996
6a4d0d2 
3996: INT & ANN: Implement intention and fix for struct fields expansion instead of `..` r=vlad20012 a=shevtsiv

<!--
Hello and thank you for the pull request!

We don't have any strict rules about pull requests, but you might check
https://github.com/intellij-rust/intellij-rust/blob/master/CONTRIBUTING.md
for some hints!

Note that we need an electronic CLA for contributions:
https://github.com/intellij-rust/intellij-rust/blob/master/CONTRIBUTING.md#cla

After you sign the CLA, please add your name to
https://github.com/intellij-rust/intellij-rust/blob/master/CONTRIBUTORS.txt

:)
-->
In this pull request I intend to do the following:
1) Create E0023 and E0027 error classes and implement analysis for them.
2) Create fix for those errors.
3) Create intention for struct fields expansion.
4) Create a module with shared code for using it in the fix and intention.

Once these steps are complete I will squash all the commits and #3928 should be closed after merge.

Co-authored-by: shevtsiv <rostykshevtsiv@gmail.com>
@bors
Copy link
Contributor

bors bot commented Aug 19, 2019

Build succeeded

@bors bors bot merged commit b0eac1c into intellij-rust:master Aug 19, 2019
@shevtsiv shevtsiv deleted the expand_fields_intention branch August 20, 2019 03:29
@ice1000 ice1000 mentioned this pull request Aug 20, 2019
Closed
ice1000
ice1000 reviewed Aug 20, 2019
View reviewed changes
src/main/kotlin/org/rust/ide/utils/StructFieldsExpander.kt
val declaration = patStruct.path.reference.deepResolve() as? RsFieldsOwner ?: return
val hasTrailingComma = patStruct.rbrace.getPrevNonCommentSibling()?.elementType == RsElementTypes.COMMA
patStruct.dotdot?.delete()
val existingFields = patStruct.patFieldList
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this patFieldList return _ patterns as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does this patFieldList return _ patterns as well?

Yes, this patFieldList also includes underscores, but the patTupleStruct.childrenOfType<RsPatIdent>() that was used below does not do this. That's why _ were considered wrong fields.

Thank you for the report!

Copy link
Contributor

Choose a reason for hiding this comment

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

but the patTupleStruct.childrenOfType() that was used below does not do this.

Oh! I didn't catch that. Thanks for your fix!

@lancelote lancelote moved this from Test to Done in To test Aug 25, 2019
@Undin Undin mentioned this pull request Sep 1, 2019
Closed
@vlad20012 vlad20012 mentioned this pull request Dec 1, 2020
Merged
bors bot added a commit that referenced this pull request Dec 7, 2020
@bors @vlad20012
Merge #6465 #6466 #6467
82339b5 
6465: ANN: support fields under `cfg` attribute in E0023 detection r=vlad20012 a=vlad20012

... and `Add missing fields` quick fix.

Improves #3996 to better work with fields under `#[cfg]`

changelog: Fix `cfg` attributes support in `E0023` detection and a corresponding quick fix.

6466: Support function parameters under cfg attributes r=ortem a=vlad20012

Fixes #6148

Correctly handle function parameters under `cfg` attributes in type inference and annotators. Works for:

```rust
fn foo(
    #[cfg(foo)]      a: u8,
    #[cfg(not(foo))] a: i8,
) {}
```

Note that it doesn't works `implement members` quick fix/action because it doesn't support any attributes at all. it should be fixed separately.

`self` parameter can also be under cfg. This case is not covered.

### Implementation details

There is `RsFunction.valueParameters` that resturns function parameter list after `cfg` evaluation and  should be used in type inference/inspections/annotatos/etc, and there is `RsFunction.rawValueParameters` that resturns function parameter list **without** `cfg` evaluation and should be used in PSI manipulations

changelog: Support function parameters under `#[cfg()]` attributes

6467: Show cfg-disabled items grayed in structure view r=Undin a=vlad20012

![image](https://user-images.githubusercontent.com/3221931/100754317-0135a480-33fc-11eb-8c5b-c1a545358046.png)


Co-authored-by: vlad20012 <beskvlad@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vlad20012 vlad20012 vlad20012 left review comments

@ice1000 ice1000 ice1000 left review comments

Assignees

@vlad20012 vlad20012

Labels
feature
Projects
To test
  
Done
Milestone
v104
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@shevtsiv @vlad20012 @ice1000 @Undin @mchernyavsky