Skip to content

Fixed IDE services touching notnull constraint#36508

Merged
AlekseyTs merged 1 commit intodotnet:masterfrom
AlekseyTs:NotNull_03
Jun 19, 2019
Merged

Fixed IDE services touching notnull constraint#36508
AlekseyTs merged 1 commit intodotnet:masterfrom
AlekseyTs:NotNull_03

Conversation

@AlekseyTs
Copy link
Copy Markdown
Contributor

No description provided.

@CyrusNajmabadi
Copy link
Copy Markdown
Contributor

Looking good. Did i miss it, or are there no completion tests here?

@AlekseyTs
Copy link
Copy Markdown
Contributor Author

Did i miss it, or are there no completion tests here?

NotNullKeywordRecommenderTests were added in #36109. Is this what you were looking for?

@CyrusNajmabadi
Copy link
Copy Markdown
Contributor

Indeed. Sorry, i missed that PR the last time around!

@jcouv jcouv self-assigned this Jun 17, 2019
End Get
End Property

Private ReadOnly Property HasNotNullConstraint As Boolean Implements ITypeParameterSymbol.HasNotNullConstraint
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.

HasNotNullConstraint [](start = 34, length = 20)

Consider adding an API test for VB (we already had tests for the C# API)

Copy link
Copy Markdown
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 (compiler and IDE changes, iteration 1)

@AlekseyTs
Copy link
Copy Markdown
Contributor Author

@dotnet/roslyn-compiler Please review, need a second sign-off.

@AlekseyTs
Copy link
Copy Markdown
Contributor Author

@dotnet/roslyn-ide Please review

@AlekseyTs
Copy link
Copy Markdown
Contributor Author

@dotnet/roslyn-compiler Please review, need a second sign-off.

@AlekseyTs
Copy link
Copy Markdown
Contributor Author

@dotnet/roslyn-ide Please review

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 changes LGTM.

@AlekseyTs
Copy link
Copy Markdown
Contributor Author

@dotnet/roslyn-ide Please review

1 similar comment
@AlekseyTs
Copy link
Copy Markdown
Contributor Author

@dotnet/roslyn-ide Please review

new NameOfKeywordRecommender(),
new NamespaceKeywordRecommender(),
new NewKeywordRecommender(),
new NotNullKeywordRecommender(),
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.

Is there a test for this?

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.

Is there a test for this?

What kind of test do you have in mind?

Copy link
Copy Markdown
Member

@genlu genlu Jun 19, 2019

Choose a reason for hiding this comment

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

Check if notnull shows as keyword in completion list

Ah, I see I have missed the comments above. Please ignore this. (although it's concerning that the test you added before didn't catch this)

Copy link
Copy Markdown
Member

@genlu genlu left a comment

Choose a reason for hiding this comment

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

:shipit:

@AlekseyTs AlekseyTs merged commit 0c2ad8a into dotnet:master Jun 19, 2019
Copy link
Copy Markdown
Member

@jasonmalinowski jasonmalinowski left a comment

Choose a reason for hiding this comment

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

Late but do want to give kudos for all the good work adding tests.

333fred added a commit to 333fred/roslyn that referenced this pull request Jun 20, 2019
* dotnet/master: (85 commits)
  Don't complete statement when typing semicolon inside comments in an argument list (dotnet#36521)
  Set focus to editor before finding text
  Add a bunch of nullability support to some code generation helpers
  Add 'annotations' and 'warnings' support to nullable directive (dotnet#36464)
  Fixed IDE services touching `notnull` constraint (dotnet#36508)
  Update compiler toolset to arcade version (dotnet#36549)
  Fix for 923157
  Do not include value types in NullableAttribute byte[] (dotnet#36519)
  Fix a deadlock in InvokeOnUIThread
  Apply a hang mitigating timeout to UI thread operations
  Move to a different lowering from for nullable value types to work around a bug in TransformCompoundAssignmentLHS. Addressed PR feedback.
  Fix stack overflow in requesting syntax directives (dotnet#36347)
  crash on ClassifyUpdate for EventFields (dotnet#35962)
  fixing bad merge with refactoring that was checked in later
  added basic completion statement telemetry
  Remove duplication in AbstractSymbolCompletionProvider.CreateItems
  Disable move type when the options service isn't present (dotnet#36334)
  Fix crash where type inference doing method inference needs to drop nullability
  Revert "Use IVsSolution to look up IVsHierarchy by project GUID (dotnet#35746)"
  Fix parsing bug in invalid using statements (dotnet#36428)
  ...
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.

7 participants