Skip to content

Only report missing disposal interfaces if used#60252

Merged
jcouv merged 3 commits intodotnet:mainfrom
jcouv:dispose-bug
Mar 19, 2022
Merged

Only report missing disposal interfaces if used#60252
jcouv merged 3 commits intodotnet:mainfrom
jcouv:dispose-bug

Conversation

@jcouv
Copy link
Copy Markdown
Member

@jcouv jcouv commented Mar 18, 2022

Fixes #45111

@jcouv jcouv self-assigned this Mar 18, 2022
@jcouv jcouv marked this pull request as ready for review March 18, 2022 04:10
@jcouv jcouv requested a review from a team as a code owner March 18, 2022 04:10
}

[Fact, WorkItem(45111, "https://github.com/dotnet/roslyn/issues/45111")]
public void IDisposableInterfaceUnnecessaryForPatternDisposal()
Copy link
Copy Markdown
Contributor

@cston cston Mar 18, 2022

Choose a reason for hiding this comment

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

Unnecessary

It looks like the interface is necessary since an error is reported. #Closed

{
void M()
{
using (null);
Copy link
Copy Markdown
Contributor

@cston cston Mar 18, 2022

Choose a reason for hiding this comment

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

;

Consider using { } to avoid the warning. #ByDesign

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is intended to be like test above, just without the declared local

}
";
var comp = CreateCompilation(source);
comp.MakeTypeMissing(WellKnownType.System_IAsyncDisposable);
Copy link
Copy Markdown
Contributor

@cston cston Mar 18, 2022

Choose a reason for hiding this comment

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

If we can test this case with System.IAsyncDisposable actually missing, we should do that, rather than hiding the type from WellKnownTypes. It looks like CreateCompilationWithTasksExtensions(source) will not include IAsyncDisposable.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Sure. I'm keeping MakeTypeMissing though, as the set of references included in our various TFMs grows over time.

Copy link
Copy Markdown
Contributor

@cston cston Mar 18, 2022

Choose a reason for hiding this comment

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

MakeTypeMissing() changes the compiler behavior though.

To verify the type is missing in the test, could we simply add Assert.Equal(TypeKind.Error, comp.GetWellKnownType(WellKnownType.System_IAsyncDisposable).TypeKind);?

@jcouv jcouv requested a review from cston March 18, 2022 07:40
@jcouv
Copy link
Copy Markdown
Member Author

jcouv commented Mar 18, 2022

@dotnet/roslyn-compiler for second review. Thanks

@jcouv
Copy link
Copy Markdown
Member Author

jcouv commented Mar 18, 2022

/azp run

@jcouv jcouv enabled auto-merge (squash) March 18, 2022 22:12
@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 4 pipeline(s).

@jcouv
Copy link
Copy Markdown
Member Author

jcouv commented Mar 19, 2022

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 4 pipeline(s).

@jcouv jcouv merged commit d343e67 into dotnet:main Mar 19, 2022
@ghost ghost added this to the Next milestone Mar 19, 2022
@allisonchou allisonchou modified the milestones: Next, 17.2.P3 Mar 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

'await using' errors if IAsyncDisposable is not defined even though it works on types that don't implement IAsyncDisposable

4 participants