First draft of an analyzer for capability APIs #111
Conversation
| This might be useful for .NET Standard library developers and much less useful | ||
| for people building .NET Core apps. We might also want different defaults. |
There was a problem hiding this comment.
Some capabilities will be less interesting for .NET Core apps, but some are as applicable as for .NET Standard libraries (I can still try to use Registry APIs when building app for Linux, it should warn). For that reason I think this should be on-by-default if we can make it performant enough.
|
|
||
| This would behave the same was tagging the assemblies, modules, types, or | ||
| members with the `[RequiredXxxCapability]` but without requiring callers to | ||
| check it. It's basically a suppression for a specific capability. |
There was a problem hiding this comment.
If it looks like suppression and behaves like suppression -> it might as well be a suppression 😄 - Why would not SuppressMessage (or the new UnconditionalSuppressMessage) work here? Maybe I don't understand the "assertion" pattern.
| This probably doesn't work in Roslyn as the diagnostic set needs to be known | ||
| statically. | ||
|
|
There was a problem hiding this comment.
In general I agree, but it would be nice if we could make at least some with distinct IDs. If nothing else it would behave nicer in combination with suppression and warning settings in IDEs.
So for example those we define in the framework could come with distinct IDs.
| return configuredPath; | ||
| } | ||
| */s | ||
| } |
There was a problem hiding this comment.
What if the code looks like:
if (Registry.IsSupported)
{ // read from registry }
else
{ throw new Exception("This feature requires registry"); }The analyzer would probably mark this as "perfectly fine", but in reality it's as broken as before. Should we consider some kind of detection of this "Anti-pattern"?
| } | ||
| ``` | ||
|
|
||
| ## Q & A |
There was a problem hiding this comment.
Should we possibly fold the proposed "linker unfriendly" attribute into this as well?: dotnet/runtime#33862
It looks like a good fit.
| } | ||
| ``` | ||
|
|
||
| ## Q & A |
There was a problem hiding this comment.
What is the handling of issues inside dependencies (NuGets mostly)? For example with the registry APIs above:
I write an app which itself doesn't call any of the registry APIs, but it uses a NuGet which does call them. Running the analyzer on the app itself will not reveal any problems, but the app is still broken on Linux.
| } | ||
| ``` | ||
|
|
||
| ## Q & A |
There was a problem hiding this comment.
We should also consider how is this related to the feature switch functionality: https://github.com/dotnet/designs/blob/master/accepted/2020/feature-switch.md
In short - linker could remove the "false" branches in lot of cases (reducing code size) - assuming this is integrated correctly.
|
It doesn't seem we're pursuing this for now. If scenarios require this, we might come back to it. |
This design isn't ready yet. I only created the draft PR so that I can point folks to it.