Skip to content

Null annotate ParameterSymbol and derived#58700

Merged
jaredpar merged 5 commits into
dotnet:mainfrom
jaredpar:param
Jan 12, 2022
Merged

Null annotate ParameterSymbol and derived#58700
jaredpar merged 5 commits into
dotnet:mainfrom
jaredpar:param

Conversation

@jaredpar
Copy link
Copy Markdown
Member

@jaredpar jaredpar commented Jan 7, 2022

No description provided.

@jaredpar jaredpar marked this pull request as ready for review January 7, 2022 15:35
@jaredpar jaredpar requested a review from a team as a code owner January 7, 2022 15:35
@jaredpar
Copy link
Copy Markdown
Member Author

jaredpar commented Jan 7, 2022

@roslyn-compiler PTAL

Comment thread src/Compilers/CSharp/Portable/Symbols/Retargeting/RetargetingParameterSymbol.cs Outdated
Copy link
Copy Markdown
Member

@333fred 333fred left a comment

Choose a reason for hiding this comment

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

Done review pass (commit 2)

Comment thread src/Compilers/CSharp/Portable/Symbols/ParameterSymbol.cs Outdated
Comment thread src/Compilers/CSharp/Portable/Symbols/Source/SourceParameterSymbolBase.cs Outdated
Comment on lines +119 to +121
/// <summary>
/// <inheritdoc/>
/// </summary>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
/// <summary>
/// <inheritdoc/>
/// </summary>
/// <inheritdoc/>

I think this is all you need, though @sharwell can correct me if I'm wrong.

@jaredpar
Copy link
Copy Markdown
Member Author

@dotnet/roslyn-compiler can I get one more review here?

}

var sourceModuleSymbol = this.ContainingModule as SourceModuleSymbol;
return (object)sourceModuleSymbol == null ? null : sourceModuleSymbol.DeclaringCompilation;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(object)

Why remove the cast?

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.

Ah good catch. I was undoing edits in this file (had temp converted it to ?.). Apparently didn't undo far enough.

@jaredpar jaredpar enabled auto-merge (squash) January 11, 2022 20:18
@jaredpar jaredpar merged commit 1b30a3b into dotnet:main Jan 12, 2022
@ghost ghost added this to the Next milestone Jan 12, 2022
@RikkiGibson RikkiGibson modified the milestones: Next, 17.2.P1 Feb 4, 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.

4 participants