Skip to content

Add 'replace' APIs and hook them up to the IDE#54270

Merged
chsienki merged 8 commits intodotnet:mainfrom
chsienki:source-generators/missing-apis
Jun 25, 2021
Merged

Add 'replace' APIs and hook them up to the IDE#54270
chsienki merged 8 commits intodotnet:mainfrom
chsienki:source-generators/missing-apis

Conversation

@chsienki
Copy link
Member

@chsienki chsienki commented Jun 21, 2021

Adds missing source generator APIs

Fixes #54087

@chsienki
Copy link
Member Author

@dotnet/roslyn-compiler & @dotnet/roslyn-ide for review please :)

Copy link
Contributor

@cston cston Jun 22, 2021

Choose a reason for hiding this comment

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

_states.Length == 2 && _states[0].IsRemoved

If Single() supports two states where the first is marked IsRemoved, are there also valid cases with more than two states where all but the last are marked IsRemoved?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sort of, but not currently in the way the NodeStateTable is used. In the cases where nodes call Single() there should ever be one element, or one removed and a new one that replaces it. We could move the stronger assertion out to the callsites, and allow the state table to contain multiple removed elements, but given that nothing else uses it right now, it seems fine to leave it here.

Copy link
Contributor

@ryzngard ryzngard left a comment

Choose a reason for hiding this comment

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

IDE changes lgtm

@jcouv jcouv self-assigned this Jun 23, 2021
Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

Done with review pass (iteration 6)

Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

LGTM Thanks (iteration 9)

@chsienki chsienki force-pushed the source-generators/missing-apis branch from 046a23b to 0d43516 Compare June 25, 2021 17:39
@chsienki chsienki merged commit f30ff9e into dotnet:main Jun 25, 2021
@ghost ghost added this to the Next milestone Jun 25, 2021
333fred added a commit that referenced this pull request Jun 25, 2021
…ures/caller-argument-expression

* upstream/main: (492 commits)
  Add nullable ref annotations to SyntaxFactory (#54199)
  Add 'replace' APIs and hook them up to the IDE (#54270)
  Allow implicit implementation of non-public interface members (#54182)
  Make insertion a stage of the official build (#54376)
  Cleanup
  Remove unused property
  Simplify glyph computation
  Report all-empty top level statements. (#54385)
  Calculate TypeParameterKind based on the container type (#54200)
  vert
  not roaming
  Split tests
  Multple matches
  Report as we get results
  Fixup tests
  Update tests
  Avoid thread dependency in VirtualMemoryNotificationListener constructor
  Fast progression search.
  Add LanguageVersion 10 (#54359)
  Sure, why not
  ...
@RikkiGibson RikkiGibson modified the milestones: Next, 17.0.P2 Jun 29, 2021
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.

Missing source generator driver incremental APIs

6 participants