Skip to content

Use property pattern#61062

Merged
CyrusNajmabadi merged 2 commits intodotnet:mainfrom
CyrusNajmabadi:propPAttern
May 3, 2022
Merged

Use property pattern#61062
CyrusNajmabadi merged 2 commits intodotnet:mainfrom
CyrusNajmabadi:propPAttern

Conversation

@CyrusNajmabadi
Copy link
Copy Markdown
Contributor

No description provided.

@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner April 30, 2022 03:15
@ghost ghost added the Area-IDE label Apr 30, 2022
@mavasani
Copy link
Copy Markdown
Contributor

Is there some analyzer/code fix to enforce this or did you manually find and fix these? If the former, can/should we enforce it on build, at least for the IDE layers to start with?

@CyrusNajmabadi
Copy link
Copy Markdown
Contributor Author

I used our analyzer. :-)

Enforcing seems totally fine to me!

@mavasani
Copy link
Copy Markdown
Contributor

Thanks, can we do the enforcement as part of this PR?

@CyrusNajmabadi
Copy link
Copy Markdown
Contributor Author

Thanks, can we do the enforcement as part of this PR?

Sure :)

@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner May 2, 2022 16:12
dotnet_diagnostic.RS0102.severity = none

# IDE0170: Prefer extended property pattern
dotnet_diagnostic.IDE0170.severity = suggestion
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

only compiler change. moving this back to a suggestion instead of a warning as i believe the compiler prefers not to enable automated style analyzers? @jcouv to confirm?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Note: if compiler is ok turning this on, i'm happy to remove this line and fix up the remaining hits in the compiler layer.

@CyrusNajmabadi
Copy link
Copy Markdown
Contributor Author

@dotnet/roslyn-compiler for tiny non-code change.

ReturnType: { SpecialType: SpecialType.System_Boolean },
Parameters: { Length: 0 },
ReturnType.SpecialType: SpecialType.System_Boolean,
Parameters.Length: 0,
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.

❔ Just wondering, is there a new pattern we would recommend for this, or is this still the cleanest?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

when list patterns come out, one might prefer Parameters: []. I could see it both ways.

Copy link
Copy Markdown
Contributor

@cston cston left a comment

Choose a reason for hiding this comment

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

Compiler change LGTM.

@CyrusNajmabadi CyrusNajmabadi merged commit 4bdc7d1 into dotnet:main May 3, 2022
@CyrusNajmabadi CyrusNajmabadi deleted the propPAttern branch May 3, 2022 23:21
@ghost ghost added this to the Next milestone May 3, 2022
@CyrusNajmabadi
Copy link
Copy Markdown
Contributor Author

Thanks all!

@Cosifne Cosifne modified the milestones: Next, 17.3 P2 May 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants