JetBrains / resharper-unity

Unity support for both ReSharper and Rider
Apache License 2.0
1.21k stars 134 forks source link

Reference assembly action should update asmdef files #852

Closed Baste-RainGames closed 2 years ago

Baste-RainGames commented 6 years ago

Rider has a very convenient "Reference 'assembly' and use 'type'" quick-fix action when you're trying to use a type from a non-referenced assembly. That kicks in for types from other asmdef-generated assemblies, but since Unity uses the asmdef file's references to generate .csproj references, the asmdef file has to be updated by hand to make the code compile in Unity.

Ideally, the reference assembly action should update the asmdef file automatically. Is that doable?

citizenmatt commented 5 years ago

This should be possible. If the reference being added is to another .asmdef based project, we should be able to do this, by implementing IModuleReferencer. This should allow us to check that the two modules can be referenced, and to do what needs to be done to make the reference happen. I suspect we'll need to modify the .asmdef, and also add the project reference, so that the type is immediately available and the quick fix completes successfully (after referencing the module, it needs to reference the type. If the module reference fails or doesn't fully complete, the type doesn't get referenced).

If the reference being added is an assembly reference, then we'll need to see if that assembly is from a .asmdef in a read only "referenced" package. If so, modify the .asmdef and add the assembly reference to the project.

You can see somewhat out of date code for this in the resharper-nuget project. It's circa ReSharper 8, and got rolled into the product for ReSharper 9. I don't think much has changed.

Unanswered questions:

Baste-RainGames commented 5 years ago

For your first question: yes, there are - all of the ones from Unity's packages system. The rules for those are:

This means that if you want to filter for assemblies that should be automatically added to an asmdef, you need to look though all other asmdef files in the project, and the manifest.json file of the project.

For your second question: seems like it. If I have a reference to assembly A in assembly B, Rider won't suggest that I add a reference to assembly B in assembly A. So that should be safe.

citizenmatt commented 5 years ago

The first question is really determining if there is ever a scenario for trying to add a purely binary reference to a .asmdef file. If so, we can't do anything in this case - a .asmdef file can only contain references to other .asmdef files.

What I mean by "purely binary reference" is an assembly that isn't part of a package, or isn't already referenced. Plugins and other .dll assets are already referenced, so we're good there. Source packages are .asmdef based, but ReSharper should be asking us to add a project reference, which we can use to easily find the .asmdef file to modify.

Cached packages are a little different. To ReSharper, they'll look like a simple binary file reference, but we need to know better. Cached packages are still .asmdef based - some packages contain multiple .asmdef files, compiling to multiple assemblies. The name in package.json is the name of the package, while the name of the assembly comes from the .asmdef file Unity used to compile it. So when we get a binary reference, we'll need to look it up in an index of .asmdef files. If we know it, we can modify the .asmdef file and add the binary assembly reference to the project (which Unity would also do as soon as the project files are regenerated).

The problem is if the binary reference doesn't match a .asmdef file. We can't fix the .asmdef file in the source package here, and we need to know if we can ever get into this situation and how to handle it. This is the scenario that needs investigating - if this scenario is possible, where does that .dll come from?

The good news is that the assembly name is the same defined name in the .asmdef file (but not the filename!) and we already have an index of the .asmdef files in the C# solution. We need to extend this for various other things, and we'll be able to reuse it here, too.

I suspect that if we're modifying a .asmdef file inside a source package (i.e. it lives in Packages or is referenced with file:// in manifest.json) then we might well need to add a package reference to package.json as well. And I'm not sure about a .dll asset that lives in Assets underneath a .asmdef file - I don't know how Unity will generate references here. These will both also need investigating.

SugoiDev commented 5 years ago

In 2018.3+, there is something like a "purely binary reference" in asmdef files. First, in the import settings of your compiled DLL, you can mark it as being "explicitly referenced".

In the UI, this is being called "Auto Reference". You need to disable it for the assembly to marked as "explicitly referenced". Here's how it looks like screenshot 2018 10 31-10 10 44

In the .meta file for that DLL, you'll have this

fileFormatVersion: 2
guid: cf2360dc86965534b80e461f9bc003ea
PluginImporter:
  (...)
  isExplicitlyReferenced: 1
  (...)

Now, you can use the "Assembly references" list in the asmdef import settings to add a reference to that DLL.

Here's how it looks like in the UI

screenshot 2018 10 31-10 10 29

and what the .asmdef file itself looks like

  (...)
    "overrideReferences": true,
    "precompiledReferences": [
        "TypeSafe.dll",
        "Enums.NET.dll",
  (...)

Note that the overrideReferences switch must be true for the precompiledReferences to be used.

DLLs with the "auto reference" switch disabled will not be added to any custom assembly unless you add them in the "assembly references" list.

I'm not sure about the "normal" Unity assemblies (non asmdef based). My guess is that the DLLs will still be referenced (since there's no way to configure the normal assemblies).

citizenmatt commented 5 years ago

Awesome! That's brilliant to know. And I guess I need to update the asmdef JSON schema, too!

NoxMortem commented 5 years ago

Oh I would love if it would correctly update the asmdef :)

Sroka commented 4 years ago

Any news on this?

citizenmatt commented 4 years ago

We're currently planning Rider 2020.3, hopefully we'll include it then. See also RIDER-48660

hybridherbst commented 3 years ago

@citizenmatt do you think this will make it to 2020.3? EAP8 doesn't seem to have it (yet). Fingers crossed 🤞

citizenmatt commented 3 years ago

Sadly no. This hasn't made it for 2020.3, it's going to be 2021.1

citizenmatt commented 3 years ago

Calling out an important detail that's easy to miss from above: consider also adding a package reference when adding an asmdef reference between two packages.

This is complicated with optional features, handled by version defines. These are symbols that are defined based on expressions that evaluate if a package is referenced, and what version of that package is referenced. They can be used to add optional features - support for a package wrapped inside defines. In this case, the .asmdef reference should be added, but the package reference is optional.

It doesn't appear to be possible to automatically decide if a package reference should be made. If a reference to a package that is already set up in version defines is required, then it might be an optional feature, or it might be version defines to allow handling multiple versions of the same package - it's still a required feature, but with optional behaviour. Similarly, an optional feature might add a reference to yet another package. It is unlikely that an optional feature should force a required reference, but it would be very hard to check that the consumer of the reference is an optional feature (optional feature or optional behaviour?). A simpler solution is to always prompt to add a package reference.

marwie commented 3 years ago

Sadly no. This hasn't made it for 2020.3, it's going to be 2021.1

Hey Matt, do you have any news on this? Using 2021.2 EAP 7 now and eager to try this :)

citizenmatt commented 3 years ago

No news, sorry. I'd like to get to this, but it's a larger piece of work that hasn't fitted in to date.

lgarczyn commented 10 months ago

This feature only works sometimes. Most of the time, the error disappears, but the asmdef is not updated. As soon as I switch to Unity and back, the error re-appears.

citizenmatt commented 10 months ago

@lgarczyn Yes, unfortunately, it's a bit buggy. We're hoping to address that in the next release, apologies for the inconvenience

lgarczyn commented 10 months ago

@lgarczyn Yes, unfortunately, it's a bit buggy. We're hoping to address that in the next release, apologies for the inconvenience

Thanks! It would be nice if the auto-fix also worked on "using" statements and extension methods, and not just missing types.

citizenmatt commented 9 months ago

Not sure what you mean by that - can you give me an example, please?

lgarczyn commented 9 months ago

Not sure what you mean by that - can you give me an example, please?

If you are missing an assembly reference, 3 things might show errors. Missing types, unrecognized namespaces and missing extension methods.

Currently, rider only offers the "auto-fix" for missing types.