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 support for .asmdef references, including GUID references #2179

Merged
merged 26 commits into from
Oct 8, 2021

Conversation

citizenmatt
Copy link
Member

This PR builds on #2174 to improve support for .asmdef files. It's currently targeting that branch, but will automatically switch to net213 when merged. Some minor conflicts will need to be addressed, since #2174 has moved on a little bit since this branch started.

  • Re-enable highlighting of resolve errors for references inside a .asmdef file. Process files from all external packages #2174 now indexes all .asmdef files from all packages, so we can resolve all references, even into builtin packages/modules. Fixes Custom PSI module for external assembly definition files #702
  • Adds support for GUID based references, which are preferable because they are based on asset GUID, rather than a name that could change. GUID references are now resolved, and work with CTRL+click and Find Usages, as well as context highlighting. These references are not affected by a rename refactoring of the assembly definition name.
  • GUID based references show the resolved name of the referenced assembly definition in a tooltip and an inlay hint. The hint can be configured from the alt+enter menu, and also from a new Unity inlay hints options page. Resolves RIDER-58568
  • Add a hint to prefer GUID based references to user-editable .asmdef files that are using name based references. The hint severity is configurable.
  • Add a scoped Quick Fix to convert name to GUID based reference, just once, or in file/folder/project/solution. Add a similar scoped context action to convert GUID references to named references.
  • Update inspection highlights to only highlight the unquoted part of a name, rather than the full quoted range
  • Fix a bug when removing an invalid item from the references array that would leave a dangling comma (e.g. when referencing self)
  • Added as many tests as possible

@citizenmatt citizenmatt added this to the Rider 2021.3 milestone Oct 6, 2021
@citizenmatt citizenmatt self-assigned this Oct 6, 2021
Base automatically changed from net213-mte-add-packages-to-module to net213 October 7, 2021 11:43
Copy link
Collaborator

@krasnotsvetov krasnotsvetov left a comment

Choose a reason for hiding this comment

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

Could you check that JetBrains.ReSharper.Plugins.Unity.JsonNew.Feature.Services.Daemon.JsonNewLanguageSpecificDaemonBehavior is applicable for asmdef files? That behaviour suppresses swea for asmdef. We are not calculating any dependencies for asmdef files so reanalyze after changes is not working for asmdef files

@citizenmatt
Copy link
Member Author

Aha, yes, it applies. And we don't get any errors in SWEA if there's an error in a .asmdef file. Should we change RunInSolutionAnalysis to return true?

@krasnotsvetov
Copy link
Collaborator

No, because asmdef is not supporting swea right now: if you create asmdef file with unknown guid, that error will be added to swea. After that, close that asmdef file and fix error by creating required asmdef, error will not disappear from solution-wide analysis, swea will not reanalyze first file, because we don't provide any info why the file should be reanalyzed

@citizenmatt
Copy link
Member Author

We've got a file watcher on all external files, and push the PSI changes through the change builder, so we're updating caches (like we're doing with assets and scenes, etc.). Is that enough for SWEA, or does it need more?

@citizenmatt citizenmatt merged commit e3e4c75 into net213 Oct 8, 2021
@citizenmatt citizenmatt deleted the net213-mte-asmdef-references branch October 8, 2021 09:24
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.

Custom PSI module for external assembly definition files
2 participants