feat: Internal attribute analyzer

[nikcio]

Prerequisites

• I have added steps to test this contribution in the description below

Description

This PR adds an analyzer and attribute UmbracoInternalAttribute that can be used to mark classes, interfaces and methods as Umbraco internal types that shouldn't be consumed outside the Umbraco repository.

This solves the problem of having public classes, interfaces and methods that needs to be public for a technical requirement like dependency injection but should in reality be internal and not messed with.

The implementation takes heavy inspiration from: https://github.com/dotnet/efcore/tree/main/src/EFCore.Analyzers and this has been included in the cs files.

This PR only adds the attribute to UmbracoDbContext and IMigrationProvider to create the tests. If these shouldn't be marked internal this can be changed.

I've also marked EnablePackageValidation as false due to it giving building errors because this would be a new package and isn't on Nuget I hope someone from HQ can help me out with if there needs to be done anything for this to work properly.

Note: The file _._ is to avoid a build error because the analyzer itself is placed outside lib/.

Test

To test the implementation I've created a simple Unit test project with some tests.

To test manually use dotnet pack locally to pack the Nuget packages (Tip: Use output parameter to place the files in a directory dotnet pack -o testRelease). Use these local versions and install it any project Console app or Umbraco template. Then reference the UmbracoDbContext and a warning should show up:

[github-actions]

Hi there @nikcio, thank you for this contribution! 👍

While we wait for one of the Core Collaborators team to have a look at your work, we wanted to let you know about that we have a checklist for some of the things we will consider during review:

• It's clear what problem this is solving, there's a connected issue or a description of what the changes do and how to test them
• The automated tests all pass (see "Checks" tab on this PR)
• The level of security for this contribution is the same or improved
• The level of performance for this contribution is the same or improved
• Avoids creating breaking changes; note that behavioral changes might also be perceived as breaking
• If this is a new feature, Umbraco HQ provided guidance on the implementation beforehand
• 💡 The contribution looks original and the contributor is presumably allowed to share it

Don't worry if you got something wrong. We like to think of a pull request as the start of a conversation, we're happy to provide guidance on improving your contribution.

If you realize that you might want to make some changes then you can do that by adding new commits to the branch you created for this work and pushing new commits. They should then automatically show up as updates to this pull request.

Thanks, from your friendly Umbraco GitHub bot 🤖 🙂

[nikcio]

It seems this reference was automatically updated when I added the new projects but the new value seems to be the right one

[busrasengul]

Thank you @nikcio
Someone from the team will look into this soon!

[nul800sebastiaan]

We'll have to discuss at HQ @nikcio - we'll get back to you! 👍

[nul800sebastiaan]

Hi @nikcio, I've had a chat with HQ and it turns out we actually have made something quite like this before: https://github.com/umbraco/Umbraco-Code/blob/master/Umbraco.Code/Volatile/UmbracoVolatileAnalyzer.cs

However, after building it we came to the conclusion that we shouldn't use it for the following reasons:

• A C# interface is expected to not change, warnings like this will likely be missed (since you don't expect them) by both implementors (people who build sites) and people who make packages
• In the newer C# releases we have default implementations that allows us to add to interfaces without breaking anyone's code

Our release cycle for major releases is only 6 months and if we need to make breaking changes to interfaces we can either add a default implementation or the change can wait until a new major version.

I know I'm a total partypooper on some of your larger PRs, so I'll encourage you once more to talk to us when you get an idea for a feature.

Also, I'm not familiar with the markdown files you added, do you have documentation for that format?

Thanks again and now on to the dozen other PRs from your hand 🎉

[nikcio]

@nul800sebastiaan The markdown files are the recommend way to track analyzer rules see: https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md

They are valid arguments. I mostly saw this as a way to signal to consumers and package developers that they are creating their implementation where they are using services that wasn't meant for consumers like @kjac mentions here: #14449 (comment)

The UmbracoDbContext is not supposed to be used outside of the Umbraco code. If anything it should probably have been marked internal rather than public - we'll have to investigate that. Bottom line though: We consider UmbracoDbContext (and any DB models eventually referred by it) for internal usage only, just as we consider the actual DB tables internal.

[nul800sebastiaan]

The markdown files are the recommend way to track analyzer rules

I don't quite understand what value they have and for whom but I'll throw it to the team to see if they think it's a practice they want to adopt.

that wasn't meant for consumers like @kjac mentions here: #14449 (comment)

Ah, I'd think we should obsolete those now then so we can make them internal when we can. 😅