Work some 'scoped' issues on IDE #65407

Youssef1313 · 2022-11-14T09:29:16Z

Related to: #64906

Fixes #65403

src/Features/CSharp/Portable/Completion/KeywordRecommenders/ScopedKeywordRecommender.cs

...ditorFeatures/CSharpTest/Completion/CompletionProviders/TypeImportCompletionProviderTests.cs

CyrusNajmabadi · 2022-11-14T19:33:55Z

@cston scoped is still in preview right?

In reply to: 1314272868

...ditorFeatures/CSharpTest/Completion/CompletionProviders/TypeImportCompletionProviderTests.cs

src/Features/CSharp/Portable/Completion/KeywordRecommenders/GlobalKeywordRecommender.cs

CyrusNajmabadi · 2022-11-14T19:37:18Z

src/Features/CSharp/Portable/Completion/KeywordRecommenders/InKeywordRecommender.cs

@@ -36,7 +36,7 @@ private static bool IsInParameterModifierContext(int position, CSharpSyntaxConte
            if (context.SyntaxTree.IsParameterModifierContext(
                    position, context.LeftToken, includeOperators: true, out var parameterIndex, out var previousModifier))
            {
-                if (previousModifier == SyntaxKind.None)
+                if (previousModifier is SyntaxKind.None or SyntaxKind.ScopedKeyword)


this feels odd. IsParameterModifierContext should understand Scoped itself right?

@CyrusNajmabadi I'm not sure what you mean?

oh, misread. sorry.

can you say this scoped?

I think yes. this scoped is valid. I'll add tests for this scoped, this ref, this in, this scoped ref, and this scoped in.

src/Features/CSharp/Portable/Completion/KeywordRecommenders/OutKeywordRecommender.cs

src/Features/CSharp/Portable/Completion/KeywordRecommenders/RefKeywordRecommender.cs

cston · 2022-11-14T19:41:49Z

@cston scoped is still in preview right?

scoped is no longer in preview; it is available in C#11 with -langversion:11.

In reply to: 1314281670

src/EditorFeatures/CSharpTest2/Recommendations/ScopedKeywordRecommenderTests.cs

CyrusNajmabadi · 2022-11-16T01:43:01Z

I can look tomorrow. Thanks!

src/EditorFeatures/CSharpTest/ReassignedVariable/CSharpReassignedVariableTests.cs

src/Features/CSharp/Portable/Completion/KeywordRecommenders/GlobalKeywordRecommender.cs

CyrusNajmabadi · 2022-11-16T22:19:22Z

src/Features/CSharp/Portable/Completion/KeywordRecommenders/ScopedKeywordRecommender.cs

+                    var previous = token.GetPreviousToken(includeSkipped: true);
+                    return previous.Kind() is SyntaxKind.ForKeyword or SyntaxKind.ForEachKeyword;


Suggested change

var previous = token.GetPreviousToken(includeSkipped: true);

return previous.Kind() is SyntaxKind.ForKeyword or SyntaxKind.ForEachKeyword;

return token.Parent is ForStatementSyntax or CommonForEachStatementSyntax;

@CyrusNajmabadi I'm following the same logic as in other keyword recommenders. I think if a change is needed here it should be in all recommenders?

CyrusNajmabadi · 2022-11-16T22:21:34Z

...haredUtilitiesAndExtensions/Workspace/CSharp/Extensions/ContextQuery/SyntaxTreeExtensions.cs

+            {
+                parent = token.Parent.Parent;
+            }
+            else if (token.IsKind(SyntaxKind.IdentifierToken) && token.Text == "scoped" && token.Parent is IdentifierNameSyntax scopedIdentifierName && scopedIdentifierName.Parent.IsKind(SyntaxKind.Parameter))


feel like there's somet repititino of these checks. but i don't feel strongly about it.

@CyrusNajmabadi Could we leave the refactoring for a separate PR?

As mentioned, I don't feel strongly about it. :-)

...haredUtilitiesAndExtensions/Workspace/CSharp/Extensions/ContextQuery/SyntaxTreeExtensions.cs

Youssef1313 · 2022-11-17T12:58:24Z

src/EditorFeatures/CSharpTest2/Recommendations/OutKeywordRecommenderTests.cs

+            await VerifyKeywordAsync("""
+                static class C
+                {
+                    static void M(this scoped $$)


out isn't valid here, but I thought it's not a big deal to offer it for simplicity?

Youssef1313 · 2022-11-17T20:30:59Z

Merging latest main and re-triggering CI because #65262 might have an impact on the logic in this PR

Youssef1313 · 2022-11-18T07:40:51Z

@CyrusNajmabadi Othan than flaky tests, everything appears to be passing. This can either be merged as is, or revised to make sure there are no dead checks after the parser changes. Unfortunately, I won't be able to validate now if there are dead checks or not.

Youssef1313 · 2022-11-22T06:52:53Z

@CyrusNajmabadi Can you restart CI here? Thanks!

This reverts commit 62646c2, reversing changes made to 35ac8cb.

#65647)" This reverts commit 1766515.

…"" (#65696)

Youssef1313 requested a review from a team as a code owner November 14, 2022 09:29

dotnet-issue-labeler bot added the Area-IDE label Nov 14, 2022

ghost added the Community The pull request was submitted by a contributor who is not a Microsoft employee. label Nov 14, 2022

Youssef1313 added 2 commits November 14, 2022 13:40

Work some 'scoped' issues on IDE

d525b85

Add auto completion for 'scoped'

fbc36c5

Youssef1313 marked this pull request as draft November 14, 2022 12:31

Adjust ref/in/out completion to work after scoped

f97b613

Youssef1313 force-pushed the scoped-ide branch from b61dab7 to f97b613 Compare November 14, 2022 13:15

Youssef1313 marked this pull request as ready for review November 14, 2022 13:17

Youssef1313 added 3 commits November 14, 2022 15:42

Fix build

60003ef

Fix build

1d08fde

Fix ref completion

1f40ee6

Youssef1313 commented Nov 14, 2022

View reviewed changes

src/Features/CSharp/Portable/Completion/KeywordRecommenders/ScopedKeywordRecommender.cs Outdated Show resolved Hide resolved

Youssef1313 added 3 commits November 14, 2022 19:49

Fix scoped completion in foreach

eb265d3

Completion for 'var' after scoped

c417ddd

More completion fixes

8b84ab7

Youssef1313 commented Nov 14, 2022

View reviewed changes

...ditorFeatures/CSharpTest/Completion/CompletionProviders/TypeImportCompletionProviderTests.cs Outdated Show resolved Hide resolved

CyrusNajmabadi reviewed Nov 14, 2022

View reviewed changes

...ditorFeatures/CSharpTest/Completion/CompletionProviders/TypeImportCompletionProviderTests.cs Outdated Show resolved Hide resolved

CyrusNajmabadi reviewed Nov 14, 2022

View reviewed changes

src/Features/CSharp/Portable/Completion/KeywordRecommenders/GlobalKeywordRecommender.cs Outdated Show resolved Hide resolved

CyrusNajmabadi reviewed Nov 14, 2022

View reviewed changes

src/Features/CSharp/Portable/Completion/KeywordRecommenders/OutKeywordRecommender.cs Show resolved Hide resolved

CyrusNajmabadi reviewed Nov 14, 2022

View reviewed changes

src/Features/CSharp/Portable/Completion/KeywordRecommenders/RefKeywordRecommender.cs Show resolved Hide resolved

Youssef1313 added 2 commits November 14, 2022 22:00

Move to CSharpCompletionCommandHandlerTests

d034331

Simplify GlobalKeywordRecommender

b060564

cston reviewed Nov 14, 2022

View reviewed changes

src/EditorFeatures/CSharpTest2/Recommendations/ScopedKeywordRecommenderTests.cs Outdated Show resolved Hide resolved

cston reviewed Nov 14, 2022

View reviewed changes

src/EditorFeatures/CSharpTest2/Recommendations/ScopedKeywordRecommenderTests.cs Outdated Show resolved Hide resolved

Address review comments

772159c

Youssef1313 requested a review from CyrusNajmabadi November 14, 2022 21:54

Merge branch 'main' into scoped-ide

646af6e

CyrusNajmabadi reviewed Nov 16, 2022

View reviewed changes

src/EditorFeatures/CSharpTest/ReassignedVariable/CSharpReassignedVariableTests.cs Outdated Show resolved Hide resolved

CyrusNajmabadi reviewed Nov 16, 2022

View reviewed changes

src/Features/CSharp/Portable/Completion/KeywordRecommenders/GlobalKeywordRecommender.cs Show resolved Hide resolved

CyrusNajmabadi reviewed Nov 16, 2022

View reviewed changes

...haredUtilitiesAndExtensions/Workspace/CSharp/Extensions/ContextQuery/SyntaxTreeExtensions.cs Outdated Show resolved Hide resolved

Youssef1313 added 2 commits November 17, 2022 14:51

Add tests, split condition

53f4568

Merge remote-tracking branch 'upstream/main' into scoped-ide

96f41c7

Youssef1313 commented Nov 17, 2022

View reviewed changes

Youssef1313 requested a review from CyrusNajmabadi November 17, 2022 13:03

Youssef1313 added 3 commits November 17, 2022 17:07

Update CSharpCompletionCommandHandlerTests.vb

322cb8c

add missing Await

72c49aa

Merge branch 'dotnet:main' into scoped-ide

a2af58b

This was referenced Nov 17, 2022

FixAllOccurrencesIgnoresGeneratedCode is flaky #56625

Open

newintegrationtests.csharp.csharpaddmissingreference.verifyavailablecodeactions (vs2022) #64807

Open

CyrusNajmabadi enabled auto-merge November 18, 2022 15:35

CyrusNajmabadi merged commit 62646c2 into dotnet:main Nov 24, 2022

ghost added this to the Next milestone Nov 24, 2022

Youssef1313 deleted the scoped-ide branch November 24, 2022 15:48

allisonchou added a commit to allisonchou/roslyn that referenced this pull request Nov 28, 2022

Revert "Merge pull request dotnet#65407 from Youssef1313/scoped-ide"

bf2b164

This reverts commit 62646c2, reversing changes made to 35ac8cb.

allisonchou added a commit that referenced this pull request Nov 28, 2022

Revert "Merge pull request #65407 from Youssef1313/scoped-ide" (#65647)

1766515

This reverts commit 62646c2, reversing changes made to 35ac8cb.

allisonchou modified the milestones: Next, 17.5 P2 Nov 30, 2022

allisonchou mentioned this pull request Dec 1, 2022

[Automated] PRs inserted in VS build main-33130.290 #65692

Closed

ryzngard added a commit that referenced this pull request Dec 1, 2022

Revert "Revert "Merge pull request #65407 from Youssef1313/scoped-ide" (

2a5025d

#65647)" This reverts commit 1766515.

ryzngard added a commit that referenced this pull request Dec 1, 2022

Revert "Revert "Merge pull request #65407 from Youssef1313/scoped-ide…

c923b2c

…"" (#65696)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Work some 'scoped' issues on IDE #65407

Work some 'scoped' issues on IDE #65407

Youssef1313 commented Nov 14, 2022

CyrusNajmabadi commented Nov 14, 2022 •

edited by jcouv

Loading

CyrusNajmabadi Nov 14, 2022

Youssef1313 Nov 14, 2022

CyrusNajmabadi Nov 16, 2022

CyrusNajmabadi Nov 16, 2022

Youssef1313 Nov 17, 2022

cston commented Nov 14, 2022 •

edited by jcouv

Loading

CyrusNajmabadi commented Nov 16, 2022

CyrusNajmabadi Nov 16, 2022

Youssef1313 Nov 17, 2022

CyrusNajmabadi Nov 16, 2022

Youssef1313 Nov 17, 2022

CyrusNajmabadi Nov 17, 2022

Youssef1313 Nov 17, 2022

Youssef1313 commented Nov 17, 2022 •

edited

Loading

Youssef1313 commented Nov 18, 2022

Youssef1313 commented Nov 22, 2022

		var previous = token.GetPreviousToken(includeSkipped: true);
		return previous.Kind() is SyntaxKind.ForKeyword or SyntaxKind.ForEachKeyword;

	var previous = token.GetPreviousToken(includeSkipped: true);
	return previous.Kind() is SyntaxKind.ForKeyword or SyntaxKind.ForEachKeyword;
	return token.Parent is ForStatementSyntax or CommonForEachStatementSyntax;

Work some 'scoped' issues on IDE #65407

Work some 'scoped' issues on IDE #65407

Conversation

Youssef1313 commented Nov 14, 2022

CyrusNajmabadi commented Nov 14, 2022 • edited by jcouv Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cston commented Nov 14, 2022 • edited by jcouv Loading

CyrusNajmabadi commented Nov 16, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Youssef1313 commented Nov 17, 2022 • edited Loading

Youssef1313 commented Nov 18, 2022

Youssef1313 commented Nov 22, 2022

CyrusNajmabadi commented Nov 14, 2022 •

edited by jcouv

Loading

cston commented Nov 14, 2022 •

edited by jcouv

Loading

Youssef1313 commented Nov 17, 2022 •

edited

Loading