ExpectedWarning silences only one warning per attribute #2479
Conversation
| var matchedMessages = loggedMessages.Where (mc => { | ||
| if (mc.Category != MessageCategory.Warning || mc.Code != expectedWarningCodeNumber) | ||
| return false; | ||
| foreach (var loggedMessage in loggedMessages) { |
There was a problem hiding this comment.
Personally I'd prefer to keep matchedMessages around, since it makes it slightly easier to debug when an attribute matches multiple warnings. Below we could either assert that only one message is matched, or remove just the first match. This also avoids having to repeat expectedWarningFound = true; loggedMessages.Remove (loggedMessage); break; in the code below.
There was a problem hiding this comment.
I guess what I didn't like about matchedMessages is that we loop over all messages (except the ones are being removed), for all attributes. If we already found a match and we know that we can remove only one logged message, why not stop there. But yeah it was convenient that matched messages allow you to run the whole logic and you could review if it matched something or not.
Notice that most messages wont repeat, and yet we will loop over all the messages looking for another match
Previously the ExpectedWarning attribute in linker silenced multiple warnings if they matched the requirements. This allowed duplicated warnings to be generated without noticing. Analyzer silences only one warning per attribute showing some bugs in linker. This PR changes the ExpectedWarning behavior in linker to only silence one warning per attribute. Commit migrated from dotnet/linker@6de0836
Previously the ExpectedWarning attribute in linker will silence multiple warnings if they matched the requirements. This allowed duplicated warnings to be generated without noticing. Analyzer silences only one warning per attribute showing some bugs in linker. This PR changes the ExpectedWarning behavior in linker to only silence one warning per attribute.