Avoid a first-chance FileNotFoundException when a ruleset include is not found. #34696
Conversation
There was a problem hiding this comment.
Is there even a reason to have a separate function here? There appears to be only a single caller.
There was a problem hiding this comment.
Agree with the premise of the change. But given there is a single caller think it would be simpler to just inline this changed method. Makes it clearer this has no other impact on the code.
There was a problem hiding this comment.
Good point, I've inlined the method.
|
@dotnet/roslyn-compiler for review |
…not found. When a .ruleset file includes a non-existing ruleset reference we receive null from ResolveIncludePath and throw a FileNotFoundException, which is the immediately caught in LoadRuleSet. We can avoid the first-chance exception and the associated allocations if we just return null. A missing ruleset is not an exceptional situation so no need to use exceptions for control flow here if we can avoid it.
KirillOsenkov
force-pushed
the
dev/kirillo/ruleset
branch
from
f6764ff to
96d5ee0
Compare
|
@mavasani is there a way to reset the integration tests? Wondering if anything else is blocking merging this. Thanks! |
|
@KirillOsenkov You can manually do a Retry by following the failure links:
I have kicked it off for you here. |
|
It's green! Who can help merge this? |
|
@agocke, did you want to take a look at this PR? |
|
Let's merge this? |
|
@KirillOsenkov Done, thanks! |
8 participants
When a .ruleset file includes a non-existing ruleset reference we receive null from ResolveIncludePath and throw a FileNotFoundException, which is the immediately caught in LoadRuleSet.
We can avoid the first-chance exception and the associated allocations if we just return null. A missing ruleset is not an exceptional situation so no need to use exceptions for control flow here if we can avoid it.
This is not blocking anything and not urgent, I was just passing by and noticed this and decided to fix.