RequiresAssemblyFilesAttribute is not always checked and applied correctly

[marek-safar]

There are cases like https://github.com/dotnet/runtime/blob/main/src/mono/System.Private.CoreLib/src/System/Reflection/RuntimeAssembly.cs#L255 where APIs which is not supported in single-file setup are not issuing any warnings when used. What is also strange is that the APIs which are annotated but have override don't need the annotation either, for example, https://github.com/dotnet/runtime/blob/main/src/mono/System.Private.CoreLib/src/System/Reflection/RuntimeAssembly.cs#L77

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[ghost]

Tagging subscribers to this area: @agocke, @vitek-karas, @VSadov
See info in area-owners.md if you want to be subscribed.

Issue Details

---

There are cases like https://github.com/dotnet/runtime/blob/main/src/mono/System.Private.CoreLib/src/System/Reflection/RuntimeAssembly.cs#L255 where APIs which is not supported in single-file setup are not issuing any warnings when used. What is also strange is that the APIs which are annotated but have override don't need the annotation either, for example, https://github.com/dotnet/runtime/blob/main/src/mono/System.Private.CoreLib/src/System/Reflection/RuntimeAssembly.cs#L77

Author:	marek-safar
Assignees:	-
Labels:	area-Single-File, untriaged
Milestone:	-

[MichalStrehovsky]

Isn't this tracked in dotnet/linker#1985? I think there's a pull request with the fix in flight.

[marek-safar]

Do you think? In this case the method is already annotated but no warning is issued (it could be build issue)

[MichalStrehovsky]

What is also strange is that the APIs which are annotated but have override don't need the annotation either,

This part sounds like the issue, but @tlakollo would know for sure. I would expect RAF to work the same as RUC in illink - all overrides have to have a matching annotation status with the slot defining method/implemented interface (so the warning here would be that CodeBase needs to have a RAF attribute and once one does that, the other warnings (at use site) will show up..

[tlakollo]

What Michal described is correct, the Requires attribute matching doesn't go through interfaces nor overrides in the analyzer at this moment, and that's true for both RUC and RAF. dotnet/linker#2046 is the PR that tracks the implementation of this functionality. With this PR we will start producing IL2046 and IL3003 to report attributes not matching with the analyzer. At this moment the PR is halted to solve the diagnostic location for a source implementation with RUC/RAF and missing attributes in a MetadataReference. Once merged we will start to have warnings on the runtime for all of these cases.

[marek-safar]

With this PR we will start producing IL2046 and IL3003 to report attributes not matching with the analyzer.

Just checking we are talking about the same thing. There are 2 places where the warning is missing now. Is your PR going to fix both of them ?

[tlakollo]

The PR will make https://github.com/dotnet/runtime/blob/main/src/mono/System.Private.CoreLib/src/System/Reflection/RuntimeAssembly.cs#L77 to warn with the new IL3003 because the override doesn't have the attribute, and once the attribute is there the analyzer will warn with IL3002 in https://github.com/dotnet/runtime/blob/main/src/mono/System.Private.CoreLib/src/System/Reflection/RuntimeAssembly.cs#L255 because the method is called. The second warning is not part of the fix in the PR, is the already existing analyzer behavior.
But in short yes, I expect after the PR is merged there will be a way to follow these broken paths and fix them

[vitek-karas]

I assume this is now fixed - the analyzer does the required checks and we added necessary annotations into the runtime in the linker->runtime integration PR.
Closing (@tlakollo please reopen if there's some work still left).