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

Fix completions after unsafe in using directive #68334

Merged
merged 5 commits into from
Jun 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// See the LICENSE file in the project root for more information.

using System;
using System.Collections.Generic;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Completion;
using Microsoft.CodeAnalysis.CSharp.Completion.Providers;
Expand Down Expand Up @@ -831,5 +832,51 @@ file class C { }

await VerifyItemExistsAsync(markup, "file");
}

[Theory, WorkItem("https://github.com/dotnet/roslyn/issues/67985")]
[MemberData(nameof(TypeDeclarationKeywords))]
public async Task TestTypeDeclarationKeywordsNotAfterUsingUnsafe(string keyword)
{
await VerifyItemIsAbsentAsync("using unsafe $$", keyword);
}

[Theory, WorkItem("https://github.com/dotnet/roslyn/issues/67985")]
[MemberData(nameof(TypeDeclarationKeywords))]
public async Task TestTypeDeclarationKeywordsNotAfterUsingStaticUnsafe(string keyword)
{
await VerifyItemIsAbsentAsync("using static unsafe $$", keyword);
}

[Theory, WorkItem("https://github.com/dotnet/roslyn/issues/67985")]
[MemberData(nameof(TypeDeclarationKeywords))]
public async Task TestTypeDeclarationKeywordsNotAfterGlobalUsingUnsafe(string keyword)
{
await VerifyItemIsAbsentAsync("global using unsafe $$", keyword);
}

[Theory, WorkItem("https://github.com/dotnet/roslyn/issues/67985")]
[MemberData(nameof(TypeDeclarationKeywords))]
public async Task TestTypeDeclarationKeywordsNotAfterGlobalUsingStaticUnsafe(string keyword)
{
await VerifyItemIsAbsentAsync("global using static unsafe $$", keyword);
}

public static IEnumerable<object[]> TypeDeclarationKeywords()
{
yield return new[] { "abstract" };
yield return new[] { "class" };
yield return new[] { "delegate" };
yield return new[] { "file" };
Copy link
Member

Choose a reason for hiding this comment

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

file would be fine. it's just a normal identifier.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this context file comes from a keyword recommender and therefore has keyword glyph. I think, this makes it more confusing than helpful. Also leaving file here would introduce a major inconsistency since neither file nor any other contextual keyword is shown in name context, where it can be treated as identifier

Copy link
Member

Choose a reason for hiding this comment

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

wfm.

note: i would far prefer these tests be in their respective provider test files. In other words, we should test 'class' in the tests for 'class'.

yield return new[] { "interface" };
yield return new[] { "internal" };
yield return new[] { "partial" };
yield return new[] { "public" };
yield return new[] { "readonly" };
yield return new[] { "record" };
yield return new[] { "ref" };
yield return new[] { "sealed" };
yield return new[] { "static" };
yield return new[] { "struct" };
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9058,6 +9058,28 @@ namespace M { }
await VerifyItemExistsAsync(markup, "M");
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/67985")]
public async Task UsingDirectives7()
{
var markup = @"
using static unsafe $$

class A { }
static class B { }

namespace N
{
class C { }
static class D { }

namespace M { }
}";

await VerifyItemExistsAsync(markup, "A");
await VerifyItemExistsAsync(markup, "B");
await VerifyItemExistsAsync(markup, "N");
}

[Fact]
public async Task UsingStaticDoesNotShowDelegates1()
{
Expand Down Expand Up @@ -9102,9 +9124,32 @@ namespace M { }
await VerifyItemExistsAsync(markup, "M");
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/67985")]
public async Task UsingStaticDoesNotShowDelegates3()
{
var markup = @"
using static unsafe $$

class A { }
delegate void B();

namespace N
{
class C { }
static class D { }

namespace M { }
}";

await VerifyItemExistsAsync(markup, "A");
await VerifyItemIsAbsentAsync(markup, "B");
await VerifyItemExistsAsync(markup, "N");
}

[Fact]
public async Task UsingStaticDoesNotShowInterfaces1()
public async Task UsingStaticShowInterfaces1()
{
// Interfaces can have implemented static methods
var markup = @"
using static N.$$

Expand All @@ -9120,13 +9165,14 @@ namespace M { }
}";

await VerifyItemExistsAsync(markup, "C");
await VerifyItemIsAbsentAsync(markup, "I");
await VerifyItemExistsAsync(markup, "I");
await VerifyItemExistsAsync(markup, "M");
}

[Fact]
public async Task UsingStaticDoesNotShowInterfaces2()
public async Task UsingStaticShowInterfaces2()
{
// Interfaces can have implemented static methods
var markup = @"
using static $$

Expand All @@ -9142,7 +9188,30 @@ namespace M { }
}";

await VerifyItemExistsAsync(markup, "A");
await VerifyItemIsAbsentAsync(markup, "I");
await VerifyItemExistsAsync(markup, "I");
await VerifyItemExistsAsync(markup, "N");
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/67985")]
public async Task UsingStaticShowInterfaces3()
{
// Interfaces can have implemented static methods
var markup = @"
using static unsafe $$

class A { }
interface I { }

namespace N
{
class C { }
static class D { }

namespace M { }
}";

await VerifyItemExistsAsync(markup, "A");
await VerifyItemExistsAsync(markup, "I");
await VerifyItemExistsAsync(markup, "N");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@ protected override bool IsValidContext(int position, CSharpSyntaxContext context
canBePartial: false,
cancellationToken: cancellationToken);

static bool ValidTypeContext(CSharpSyntaxContext context)
bool ValidTypeContext(CSharpSyntaxContext context)
=> (context.IsNonAttributeExpressionContext || context.IsTypeContext)
&& !context.IsConstantExpressionContext
&& !context.LeftToken.IsTopLevelOfUsingAliasDirective();
&& !context.SyntaxTree.IsUsingStaticContext(position, cancellationToken);
}

private static bool IsAfterAsyncKeywordInExpressionContext(CSharpSyntaxContext context, CancellationToken cancellationToken)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -261,9 +261,9 @@ private ImmutableArray<ISymbol> GetSymbolsForTypeOrNamespaceContext()
return symbols.WhereAsArray(s => s.IsNamespace());
}

if (_context.TargetToken.IsStaticKeywordInUsingDirective())
if (_context.TargetToken.IsStaticKeywordContextInUsingDirective())
{
return symbols.WhereAsArray(s => !s.IsDelegateType() && !s.IsInterfaceType());
return symbols.WhereAsArray(s => !s.IsDelegateType());
}

return symbols;
Expand Down Expand Up @@ -362,7 +362,7 @@ private RecommendedSymbols GetSymbolsOffOfName(NameSyntax name)
if (usingDirective != null && usingDirective.Alias == null)
{
return new RecommendedSymbols(usingDirective.StaticKeyword.IsKind(SyntaxKind.StaticKeyword)
? symbols.WhereAsArray(s => !s.IsDelegateType() && !s.IsInterfaceType())
? symbols.WhereAsArray(s => !s.IsDelegateType())
: symbols.WhereAsArray(s => s.IsNamespace()));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1045,14 +1045,6 @@ public static bool IsInDeconstructionLeft(
return false;
}

public static bool IsTopLevelOfUsingAliasDirective(this SyntaxToken node)
=> node switch
{
{ Parent: NameEqualsSyntax { Parent: UsingDirectiveSyntax _ } } => true,
{ Parent: IdentifierNameSyntax { Parent: UsingDirectiveSyntax _ } } => true,
_ => false
};

public static T WithCommentsFrom<T>(this T node, SyntaxToken leadingToken, SyntaxToken trailingToken)
where T : SyntaxNode
=> node.WithCommentsFrom(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ public bool IsTypeAttributeContext(CancellationToken cancellationToken)
if (token.Kind() == SyntaxKind.OpenBracketToken &&
token.Parent.IsKind(SyntaxKind.AttributeList) &&
this.SyntaxTree.IsTypeDeclarationContext(
token.SpanStart, contextOpt: null, validModifiers: null, validTypeDeclarations: SyntaxKindSet.ClassInterfaceStructRecordTypeDeclarations, canBePartial: false, cancellationToken: cancellationToken))
token.SpanStart, context: null, validModifiers: null, validTypeDeclarations: SyntaxKindSet.ClassInterfaceStructRecordTypeDeclarations, canBePartial: false, cancellationToken: cancellationToken))
{
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,19 @@ public static bool IsUsingKeywordInUsingDirective(this SyntaxToken token)
return false;
}

public static bool IsStaticKeywordInUsingDirective(this SyntaxToken token)
public static bool IsStaticKeywordContextInUsingDirective(this SyntaxToken token)
{
if (token.IsKind(SyntaxKind.StaticKeyword))
// using static |
if (token is { RawKind: (int)SyntaxKind.StaticKeyword, Parent: UsingDirectiveSyntax })
{
var usingDirective = token.GetAncestor<UsingDirectiveSyntax>();
if (usingDirective != null &&
usingDirective.StaticKeyword == token)
{
return true;
}
return true;
}

// using static unsafe |
if (token.IsKind(SyntaxKind.UnsafeKeyword) &&
token.GetPreviousToken() is { RawKind: (int)SyntaxKind.StaticKeyword, Parent: UsingDirectiveSyntax })
{
return true;
}

return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -527,16 +527,16 @@ public static bool IsTypeDeclarationContext(
public static bool IsTypeDeclarationContext(
this SyntaxTree syntaxTree,
int position,
CSharpSyntaxContext? contextOpt,
CSharpSyntaxContext? context,
ISet<SyntaxKind>? validModifiers,
ISet<SyntaxKind>? validTypeDeclarations,
bool canBePartial,
CancellationToken cancellationToken)
{
// We only allow nested types inside a class, struct, or interface, not inside a
// an enum.
var typeDecl = contextOpt != null
? contextOpt.ContainingTypeDeclaration
var typeDecl = context != null
? context.ContainingTypeDeclaration
: syntaxTree.GetContainingTypeDeclaration(position, cancellationToken);

validTypeDeclarations ??= SpecializedCollections.EmptySet<SyntaxKind>();
Expand All @@ -550,14 +550,14 @@ public static bool IsTypeDeclarationContext(
}

// Check many of the simple cases first.
var leftToken = contextOpt != null
? contextOpt.LeftToken
var leftToken = context != null
? context.LeftToken
: syntaxTree.FindTokenOnLeftOfPosition(position, cancellationToken);

// If we're touching the right of an identifier, move back to
// previous token.
var token = contextOpt != null
? contextOpt.TargetToken
var token = context != null
? context.TargetToken
: leftToken.GetPreviousTokenIfTouchingWord(position);

if (token.IsAnyAccessorDeclarationContext(position))
Expand All @@ -577,13 +577,13 @@ public static bool IsTypeDeclarationContext(
return true;
}

// using static | is never a type declaration context
if (token.IsStaticKeywordInUsingDirective())
// using directive is never a type declaration context
if (token.GetAncestor<UsingDirectiveSyntax>() is not null)
{
return false;
}

var modifierTokens = contextOpt?.PrecedingModifiers ?? syntaxTree.GetPrecedingModifiers(position, cancellationToken);
var modifierTokens = context?.PrecedingModifiers ?? syntaxTree.GetPrecedingModifiers(position, cancellationToken);
if (modifierTokens.IsEmpty())
return false;

Expand Down Expand Up @@ -654,7 +654,8 @@ public static bool IsNamespaceContext(
}

// using static |
if (token.IsStaticKeywordInUsingDirective())
// using static unsafe |
if (token.IsStaticKeywordContextInUsingDirective())
{
return true;
}
Expand Down Expand Up @@ -803,11 +804,12 @@ public static bool IsUsingAliasTypeContext(this SyntaxTree syntaxTree, int posit
public static bool IsUsingStaticContext(this SyntaxTree syntaxTree, int position, CancellationToken cancellationToken)
{
// using static |
// using static unsafe |

var token = syntaxTree.FindTokenOnLeftOfPosition(position, cancellationToken);
token = token.GetPreviousTokenIfTouchingWord(position);

return token.IsStaticKeywordInUsingDirective();
return token.IsStaticKeywordContextInUsingDirective();
}

public static bool IsTypeArgumentOfConstraintClause(
Expand Down