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

Disable namespace provider for Packages and subfolder #1677

Merged
merged 4 commits into from
Jun 4, 2020

Conversation

citizenmatt
Copy link
Member

@citizenmatt citizenmatt commented May 28, 2020

Add new rules to disable the "namespace provider" for the following folders. This will stop Rider/ReSharper from suggesting that folder as part of the namespace name. Once this PR is applied, the following folders will no longer be included in namespace name suggestions:

  • Assets
  • Assets/Scripts
  • Packages/<package-root> - any top level folder under Packages
  • Packages/<package-root>/** - any non-project folders. I.e. any folder under a package root that does not have package.json
  • Packages/<package-root>/**/Runtime
  • Packages/<package-root>/**/Scripts
  • Packages/<package-root>/**/Runtime/Scripts
  • Packages/<package-root>/**/Scripts/Runtime
  • Library/PackageCache/<package-root>
  • Library/PackageCache/<package-root>/** - any non-project folders. I.e. any folder under a package root that does not have package.json
  • Library/PackageCache/<package-root>/**/Runtime
  • Library/PackageCache/<package-root>/**/Scripts
  • Library/PackageCache/<package-root>/**/Runtime/Scripts
  • Library/PackageCache/<package-root>/**/Scripts/Runtime

This means that code in packages do not get the package name suggested, nor Scripts, Runtime, or a combination of the two. The patterns will also exclude any folder at the root of a package folder that does not contain package.json. This matches the use case of a git repo added to the Packages folder that does not have a package in the root folder of the repo (the package must be manually added via a file: reference in Packages/manifest.json)

The Library/PackageCache patterns stop ReSharper/Rider prompting if the user generates projects for referenced packages (which is arguably a bad idea, as these files should not be user editable, and are not in source control).

This does not fix all incorrect warnings for mismatched namespaces, as there are no widely accepted standards or conventions for namespaces or folder structure, but it does remove a number of poor suggestions.

It is not currently possible to set a root namespace for packages (although this will be fixed in Unity 2020.2), so it is hard to get correct suggestions. The current recommendation is that the root folder of the assembly (not the package) has the same name as the root namespace. E.g. Packages/com.unity.collections@0.0.9-preview.9/Unity.Collections/UnityCollections.asmdef would have a root namespace of Unity.Collections. These changes support this scheme.

Fixes #1161 and addresses #1584 and RIDER-36546

@citizenmatt citizenmatt added this to the Rider 2020.2 milestone May 28, 2020
@citizenmatt citizenmatt requested a review from van800 May 28, 2020 19:54
@citizenmatt citizenmatt self-assigned this May 28, 2020
@citizenmatt citizenmatt marked this pull request as ready for review May 28, 2020 19:56
@van800
Copy link
Contributor

van800 commented May 30, 2020

If we remove Packages and its subfolder, then here preferred namespace would be just Editor
image

@citizenmatt
Copy link
Member Author

There's no agreed standard for namespaces or folder layouts, which makes this tricky. We disable it for Assets, so it makes sense that we disable it for Packages too. The sub folder is the name of the package, but not necessarily the name of the assembly/project, especially if the package includes multiple asmdef files, such as for tests.

It doesn't work for this example, but it does work for the scenario of taking one of Unity's own packages and copying it from Library/PackageCache to the Packages folder. E.g. the Unity Collections package has a root folder of Packages/com.unity.collections, but the main assembly is in Packages/com.unity.collections/Unity.Collections. The namespace in the code is Unity.Collections. The same happens with Packages/com.unity.mathematics/Unity.Mathematics.

On the other hand, the assembly defined in Packages/com.unity.ads/Runtime/Advertisement has a namespace of UnityEngine.Advertisements, which we couldn't derive from the folder names.

I don't think we can get this right 100% of the time, but we can set up a good default, and I think removing the folder location and the package name is a good start. If the name of the root folder of the actual assembly is the required root namespace (Packages/com.unity.mathematics/Unity.Mathematics) then it works.

@citizenmatt
Copy link
Member Author

And that's made me think that if we generate projects for all packages, we should take a look at what happens for packages in Library/PackageCache/<package_name> too. Perhaps this also needs to be excluded.

@van800
Copy link
Contributor

van800 commented May 30, 2020

You were right, that's a special case.

image

@citizenmatt
Copy link
Member Author

I've excluded Library/PackageCache/<pacakge-name@version> now as well. Looking at an example project with a load of referenced packages, I'm tempted to remove Packages/<package_name>/Runtime, too. I've never seen it in a namespace, and it seems to be there to mirror the Editor folder. What do you think?

This handles the edge case of a git repo in the Packages folder that does not have a package in the repo root folder
@citizenmatt citizenmatt merged commit 71aa96c into net202 Jun 4, 2020
@citizenmatt citizenmatt deleted the feature/package-namespace-provider branch June 4, 2020 18:04
@syscrusher1709
Copy link

Thanks for this set of changes; I think this is an improvement.

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.

Consider stopping {package_root}/Scripts acting as a namespace provider
3 participants