Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attempt to mark EqualityContract property accessor as not auto-implemented #57917

Merged
merged 7 commits into from
Nov 30, 2021

Conversation

Youssef1313
Copy link
Member

@Youssef1313 Youssef1313 commented Nov 22, 2021

No idea if this will fail something. But at a first glance it looks like it shouldn't be an auto implemented property accessor. The property itself isn't auto property, why would the accessor be?

The motivation for the change here is to make isAutoPropertyAccessor always matching property.IsAutoProperty so we can actually be able to remove the parameter entirely.

This is one step towards making #57915 pass, which will make the life much easier for semi auto props.

@AlekseyTs for review.

@ghost ghost added the Community The pull request was submitted by a contributor who is not a Microsoft employee. label Nov 22, 2021
@Youssef1313

This comment has been minimized.

@AlekseyTs
Copy link
Contributor

Done with review pass (commit 2)

@Youssef1313 Youssef1313 marked this pull request as ready for review November 30, 2021 12:18
@Youssef1313 Youssef1313 requested a review from a team as a code owner November 30, 2021 12:18
@AlekseyTs
Copy link
Contributor

Done with review pass (commit 5).

@jcouv jcouv added the Feature - Records Records label Nov 30, 2021
Copy link
Contributor

@AlekseyTs AlekseyTs left a comment

Choose a reason for hiding this comment

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

LGTM (commit 7), assuming CI is passing.

@AlekseyTs AlekseyTs requested a review from a team November 30, 2021 18:25
@AlekseyTs
Copy link
Contributor

@dotnet/roslyn-compiler For a second review.

Copy link
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.

LGTM (commit 7)

@333fred 333fred enabled auto-merge (squash) November 30, 2021 18:50
@333fred 333fred merged commit f068f44 into dotnet:main Nov 30, 2021
@ghost ghost added this to the Next milestone Nov 30, 2021
@Youssef1313 Youssef1313 deleted the patch-12 branch November 30, 2021 20:36
@allisonchou allisonchou modified the milestones: Next, 17.1.P2 Nov 30, 2021
333fred added a commit to 333fred/roslyn that referenced this pull request Dec 3, 2021
…rovements

* upstream/main: (310 commits)
  Read SourceLink info and call service to retrieve source from there (dotnet#57978)
  Add new parser/lexer to the StackTraceAnalyzer (dotnet#57598) (dotnet#58050)
  Snap 17.1 P2 (dotnet#58041)
  Make it possible to analyze the dataflow of `ConstructorInitializerSyntax` and `PrimaryConstructorBaseTypeSyntax` (dotnet#57576)
  Shorten paths in VS installation (dotnet#57726)
  Add comments
  Add new parser/lexer to the StackTraceAnalyzer (dotnet#57598)
  Fix await completion for expression body lambda
  Add tests
  Fix comment
  Honor option, and also improve formatting with comment
  Skip TestLargeStringConcatenation (dotnet#58035)
  Log runtime framework of remote host
  Mark EqualityContract property accessor as not auto-implemented (dotnet#57917)
  Fix typo in XML doc for GeneratorExtensions (dotnet#58020)
  Hold Receiver directly in bound node for implicit indexer access (dotnet#58009)
  Pass AnalysisKind instead of int
  Enable nullable reference types for TableDataSource
  Simplify 'interpolation' data, and move to an easier to consume System.Range approach for it (dotnet#57966)
  Add missing test for CallerArgumentExpression (dotnet#57805)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Community The pull request was submitted by a contributor who is not a Microsoft employee. Feature - Records Records
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants