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

INSP: Add error for repr without parentheses #6352

Merged
merged 3 commits into from Nov 16, 2020
Merged

INSP: Add error for repr without parentheses #6352

merged 3 commits into from Nov 16, 2020

Conversation

zeroeightysix
Copy link
Contributor

@zeroeightysix zeroeightysix commented Nov 4, 2020 •
edited by Undin

Adds a new error for when a repr attribute does not have RsMetaItemArgs (thus, no pair of parentheses)

image

Fixes #6336

changelog: Show error if repr attribute doesn't have parentheses and provide quick-fix to add them

@zeroeightysix
Copy link
Contributor Author

Draft because I'm not sure whether or not to add a Add parentheses fix in this PR, or another one. What would you prefer?

@Undin
Copy link
Member

Undin commented Nov 9, 2020

@zeroeightysix I think Add parentheses quick fix is quite simple so you can put it into this PR

@zeroeightysix zeroeightysix marked this pull request as ready for review November 9, 2020 21:43
@Undin Undin added the feature label Nov 13, 2020
@Undin Undin self-assigned this Nov 13, 2020
Undin
Undin reviewed Nov 15, 2020
View reviewed changes
Copy link
Member

@Undin Undin left a comment

Choose a reason for hiding this comment

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

In general looks ok except some comments.

Since this diagnostic can be quite easily applied for different attributes, I've added some optional comments about generalization. If you desired to improve it, you may fix these comments in separate PR with supporting more attributes

src/test/kotlin/org/rust/ide/annotator/fixes/AddReprParenthesesFixTest.kt Outdated Show resolved Hide resolved
src/main/kotlin/org/rust/ide/annotator/fixes/AddReprParenthesesFix.kt Outdated Show resolved Hide resolved
src/main/kotlin/org/rust/ide/annotator/fixes/AddReprParenthesesFix.kt Outdated Show resolved Hide resolved
src/main/kotlin/org/rust/ide/annotator/fixes/AddReprParenthesesFix.kt Outdated Show resolved Hide resolved
src/main/kotlin/org/rust/ide/annotator/fixes/AddReprParenthesesFix.kt Outdated Show resolved Hide resolved
src/main/kotlin/org/rust/ide/annotator/RsErrorAnnotator.kt Outdated Show resolved Hide resolved
src/main/kotlin/org/rust/ide/annotator/fixes/AddReprParenthesesFix.kt Outdated Show resolved Hide resolved
src/main/kotlin/org/rust/lang/utils/RsDiagnostic.kt Outdated Show resolved Hide resolved
Undin
Undin reviewed Nov 16, 2020
View reviewed changes
Copy link
Member

@Undin Undin left a comment

Choose a reason for hiding this comment

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

LGTM except comments

Could you also squash all "fixing" commits into main ones, please?

src/main/kotlin/org/rust/ide/annotator/fixes/AddAttrParenthesesFix.kt Outdated Show resolved Hide resolved
src/main/kotlin/org/rust/ide/annotator/fixes/AddAttrParenthesesFix.kt Outdated Show resolved Hide resolved
src/main/kotlin/org/rust/lang/utils/RsDiagnostic.kt Outdated Show resolved Hide resolved
@Undin Undin added this to the v136 milestone Nov 16, 2020
Undin
Undin approved these changes Nov 16, 2020
View reviewed changes
Copy link
Member

@Undin Undin left a comment

Choose a reason for hiding this comment

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

Great!
Thank you!

bors r+

@Undin
Copy link
Member

Undin commented Nov 16, 2020

@zeroeightysix Would you like to extend this diagnostic for other standard attributes like allow, derive, etc?

@zeroeightysix
Copy link
Contributor Author

@zeroeightysix Would you like to extend this diagnostic for other standard attributes like allow, derive, etc?

Surely, but in another PR if it's okay with you.

@Undin
Copy link
Member

Undin commented Nov 16, 2020

@zeroeightysix 👍

Surely, but in another PR if it's okay with you.

Yep, of course. bors is already making final checks to merge this PR into master

@bors
Copy link
Contributor

bors bot commented Nov 16, 2020

Build succeeded:

@bors bors bot merged commit 8a42846 into intellij-rust:master Nov 16, 2020
@zeroeightysix zeroeightysix deleted the error/repr-noattrib branch November 18, 2020 23:24
@zeroeightysix zeroeightysix mentioned this pull request Nov 19, 2020
Merged
bors bot added a commit that referenced this pull request Nov 23, 2020
@bors @dima74 @zeroeightysix
Merge #6398 #6413
da86d17 
6398: RES: Fix exception when resolving import in cfg-disabled shadowed mod using new resolve r=vlad20012 a=dima74

Fixes exception in [new resolve](#6217) when we resolve import in shadowed mod (not accessible from `CrateDefMap` root because of some other mod with same `crateRelativePath`). We already ignore such imports if they are collected during collecting explicitly declared items. In this PR we also ignore imports added by macro expansion.

<details>

```
java.lang.AssertionError: Can't find ModData for process::imp::reap in crate tokio(lib)
	at org.rust.lang.core.resolve2.CrateDefMap.doGetModData(CrateDefMap.kt:466)
	at org.rust.lang.core.resolve2.CrateDefMap.getModData(CrateDefMap.kt:75)
	at org.rust.lang.core.resolve2.CrateDefMap.tryCastToModData(CrateDefMap.kt:88)
	at org.rust.lang.core.resolve2.PathResolutionKt.resolvePathFp(PathResolution.kt:67)
	at org.rust.lang.core.resolve2.DefCollector.resolveImport(DefCollector.kt:108)
	at org.rust.lang.core.resolve2.DefCollector.resolveImports(DefCollector.kt:74)
	at org.rust.lang.core.resolve2.DefCollector.collect(DefCollector.kt:59)
	at org.rust.lang.core.resolve2.FacadeBuildDefMapKt.buildDefMap(FacadeBuildDefMap.kt:31)
	at org.rust.lang.core.resolve2.DefMapsBuilder.doBuildDefMap(DefMapsBuilder.kt:110)
	at org.rust.lang.core.resolve2.DefMapsBuilder.buildSync(DefMapsBuilder.kt:79)
	at org.rust.lang.core.resolve2.DefMapsBuilder.build(DefMapsBuilder.kt:54)
	at org.rust.lang.core.resolve2.DefMapUpdater.doRun(FacadeUpdateDefMap.kt:159)
	at org.rust.lang.core.resolve2.DefMapUpdater.runWithStrongReferencesToDefMapHolders(FacadeUpdateDefMap.kt:135)
	at org.rust.lang.core.resolve2.DefMapUpdater.access$runWithStrongReferencesToDefMapHolders(FacadeUpdateDefMap.kt:93)
	at org.rust.lang.core.resolve2.DefMapUpdater$run$$inlined$measureTimeMillis$lambda$1.invoke(FacadeUpdateDefMap.kt:120)
	at org.rust.lang.core.resolve2.DefMapUpdater$run$$inlined$measureTimeMillis$lambda$1.invoke(FacadeUpdateDefMap.kt:93)
	at org.rust.openapiext.UtilsKt$executeUnderProgress$1.run(utils.kt:313)
	at com.intellij.openapi.progress.impl.CoreProgressManager.executeProcessUnderProgress(CoreProgressManager.java:617)
	at com.intellij.openapi.progress.impl.ProgressManagerImpl.executeProcessUnderProgress(ProgressManagerImpl.java:65)
	at org.rust.openapiext.UtilsKt.executeUnderProgress(utils.kt:313)
	at org.rust.lang.core.resolve2.DefMapUpdater.run(FacadeUpdateDefMap.kt:119)
	at org.rust.lang.core.resolve2.FacadeUpdateDefMapKt$updateDefMapForAllCrates$1$1.invoke(FacadeUpdateDefMap.kt:73)
	at org.rust.lang.core.resolve2.FacadeUpdateDefMapKt$updateDefMapForAllCrates$1$1.invoke(FacadeUpdateDefMap.kt)
	at org.rust.lang.core.resolve2.FacadeUpdateDefMapKt$sam$com_intellij_openapi_util_ThrowableComputable$0.compute(FacadeUpdateDefMap.kt)
	at com.intellij.openapi.progress.util.ProgressIndicatorUtils.computeWithLockAndCheckingCanceled(ProgressIndicatorUtils.java:326)
	at org.rust.lang.core.resolve2.FacadeUpdateDefMapKt.withLockAndCheckingCancelled(FacadeUpdateDefMap.kt:265)
	at org.rust.lang.core.resolve2.FacadeUpdateDefMapKt.withLockAndCheckingCancelled$default(FacadeUpdateDefMap.kt:264)
	at org.rust.lang.core.resolve2.FacadeUpdateDefMapKt$updateDefMapForAllCrates$1.invoke(FacadeUpdateDefMap.kt:71)
	at org.rust.lang.core.resolve2.FacadeUpdateDefMapKt$updateDefMapForAllCrates$1.invoke(FacadeUpdateDefMap.kt)
	at org.rust.openapiext.UtilsKt$runReadActionInSmartMode$1.compute(utils.kt:275)
	at com.intellij.openapi.project.DumbService.lambda$runReadActionInSmartMode$0(DumbService.java:103)
	at com.intellij.openapi.project.DumbService.lambda$runReadActionInSmartMode$1(DumbService.java:147)
	at com.intellij.openapi.application.impl.ApplicationImpl.runReadAction(ApplicationImpl.java:889)
	at com.intellij.openapi.application.ReadAction.compute(ReadAction.java:61)
	at com.intellij.openapi.project.DumbService.runReadActionInSmartMode(DumbService.java:140)
	at com.intellij.openapi.project.DumbService.runReadActionInSmartMode(DumbService.java:103)
	at org.rust.openapiext.UtilsKt.runReadActionInSmartMode(utils.kt:273)
	at org.rust.lang.core.resolve2.FacadeUpdateDefMapKt.updateDefMapForAllCrates(FacadeUpdateDefMap.kt:70)
	at org.rust.lang.core.resolve2.FacadeUpdateDefMapKt.updateDefMapForAllCrates$default(FacadeUpdateDefMap.kt:64)
	at org.rust.lang.core.macros.MacroExpansionTaskBase.run(MacroExpansionTask.kt:97)
```

</details>

6413: INSP: Detect attributes that require parentheses but have no parentheses. r=Undin a=zeroeightysix

This PR builds on #6352, making the diagnostic and fix work for all attributes that 'require parentheses', e.g. are not valid as just a name (like `#[test]`).

![image](https://user-images.githubusercontent.com/27009727/99704983-694dc780-2a99-11eb-9bd1-1edb54b32a4f.png)

changelog: Detect attributes that require parentheses but have no parentheses, and suggest `Add parentheses` quick-fix

Co-authored-by: Dmitry Murzin <diralik@yandex.ru>
Co-authored-by: Ridan Vandenbergh <ridanvandenbergh@gmail.com>
bors bot added a commit that referenced this pull request Nov 23, 2020
@bors @zeroeightysix
Merge #6413
5e10572 
6413: INSP: Detect attributes that require parentheses but have no parentheses. r=Undin a=zeroeightysix

This PR builds on #6352, making the diagnostic and fix work for all attributes that 'require parentheses', e.g. are not valid as just a name (like `#[test]`).

![image](https://user-images.githubusercontent.com/27009727/99704983-694dc780-2a99-11eb-9bd1-1edb54b32a4f.png)

changelog: Detect attributes that require parentheses but have no parentheses, and suggest `Add parentheses` quick-fix

Co-authored-by: Ridan Vandenbergh <ridanvandenbergh@gmail.com>
@Undin Undin mentioned this pull request Nov 29, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Stzx Stzx Stzx requested changes

@Undin Undin Undin approved these changes

Assignees

@Undin Undin

Labels
feature
Projects
None yet
Milestone
v136
Development

Successfully merging this pull request may close these issues.

Show error on repr attribute without parenthesis
3 participants
@zeroeightysix @Undin @Stzx