Skip to content

Replace linker suppressions with attributes #359

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

Merged
merged 3 commits into from
Aug 3, 2022

Conversation

Sergio0694
Copy link
Member

@Sergio0694 Sergio0694 commented Aug 2, 2022

As mentioned by @vitek-karas here (dotnet/linker#2938 (comment)), using #pragma warning disable for linker annotations is incorrect, as it won't prevent those warnings from actually being emitted. This PR removes the few the MVVM Toolkit had, and just adds the proper annotations to remove the warnings.

cc. @kant2002

PR Checklist

  • Created a feature/dev branch in your fork (vs. submitting directly from a commit on main)
  • Based off latest main branch of toolkit
  • PR doesn't include merge commits (always rebase on top of our main, if needed)
  • Tested code with current supported SDKs
  • Header has been added to all new source files (run build/UpdateHeaders.bat)
  • Contains NO breaking changes
  • Every new API (including internal ones) has full XML docs
  • Code follows all style conventions

@Sergio0694 Sergio0694 added the bugfix 🔧 PRs fixing a discovered bug label Aug 2, 2022
Comment on lines 615 to +619
// Get or compute the cached list of properties to validate. Here we're using a static lambda to ensure the
// delegate is cached by the C# compiler, see the related issue at https://github.com/dotnet/roslyn/issues/5835.
EntityValidatorMap.GetValue(GetType(), static t => GetValidationAction(t))(this);
EntityValidatorMap.GetValue(
GetType(),
[RequiresUnreferencedCode("The type of the current instance cannot be statically discovered.")] static (t) => GetValidationAction(t))(this);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can now just write GetValidationAction. The linked Roslyn issue has been closed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue is closed but the fix is slated for the C# 11 timeslot, I don't think it's there in VS stable just yet, meaning our CI will be using an older version of Roslyn too without that. I would keep that as is for now (especially given it's unrelated from this PR anyway), and we can update this later on when we bump the SDK as well 🙂

@Sergio0694 Sergio0694 merged commit 96517ea into main Aug 3, 2022
@delete-merged-branch delete-merged-branch bot deleted the dev/fix-linker-pragma-suppressions branch August 3, 2022 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix 🔧 PRs fixing a discovered bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants