Skip to content

Commit

Permalink
Merge pull request #6358 from Youssef1313/using-system
Browse files Browse the repository at this point in the history
Don't import System when it's already imported by global using
  • Loading branch information
mavasani authored Dec 21, 2022
2 parents f10e7f2 + fde7601 commit f983f0e
Show file tree
Hide file tree
Showing 9 changed files with 48 additions and 113 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using System.Collections.Generic;
using System.Composition;
using System.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeFixes;
Expand All @@ -11,7 +11,7 @@

namespace Microsoft.NetCore.CSharp.Analyzers.Runtime
{
[ExportCodeFixProvider(LanguageNames.CSharp)]
[ExportCodeFixProvider(LanguageNames.CSharp), Shared]
public sealed class CSharpPreferStreamAsyncMemoryOverloadsFixer : PreferStreamAsyncMemoryOverloadsFixer
{
protected override SyntaxNode? GetArgumentByPositionOrName(IInvocationOperation invocation, int index, string name, out bool isNamed)
Expand Down Expand Up @@ -54,19 +54,6 @@ public sealed class CSharpPreferStreamAsyncMemoryOverloadsFixer : PreferStreamAs
return null;
}

protected override bool IsSystemNamespaceImported(IReadOnlyList<SyntaxNode> importList)
{
foreach (SyntaxNode import in importList)
{
if (import is UsingDirectiveSyntax { Name: IdentifierNameSyntax { Identifier.Text: nameof(System) } })
{
return true;
}
}

return false;
}

protected override bool IsPassingZeroAndBufferLength(SemanticModel model, SyntaxNode bufferValueNode, SyntaxNode offsetValueNode, SyntaxNode countValueNode)
{
// First argument should be an identifier name node
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using System.Collections.Generic;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.CSharp.Syntax;
Expand All @@ -21,17 +20,6 @@ private protected override SyntaxNode ReplaceInvocationMethodName(SyntaxGenerato
return invocationSyntax.ReplaceNode(oldNameSyntax, newNameSyntax);
}

private protected override bool IsSystemNamespaceImported(Project project, IReadOnlyList<SyntaxNode> namespaceImports)
{
foreach (var import in namespaceImports)
{
if (import is UsingDirectiveSyntax { Name: IdentifierNameSyntax { Identifier.ValueText: nameof(System) } })
return true;
}

return false;
}

private protected override IOperation WalkDownBuiltInImplicitConversionOnConcatOperand(IOperation operand)
{
return UseSpanBasedStringConcat.CSharpWalkDownBuiltInImplicitConversionOnConcatOperand(operand);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using System.Collections.Generic;
using System.Collections.Immutable;
using System.Threading;
using System.Threading.Tasks;
using Analyzer.Utilities;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CodeFixes;
Expand All @@ -30,12 +30,11 @@ namespace Microsoft.NetCore.Analyzers.Runtime
/// </summary>
public abstract class PreferStreamAsyncMemoryOverloadsFixer : CodeFixProvider
{
private static readonly SyntaxAnnotation s_asMemorySymbolAnnotation = new("SymbolId", "System.MemoryExtensions");

// Checks if the argument in the specified index has a name. If it doesn't, returns that arguments. If it does, then looks for the argument using the specified name, and returns it, or null if not found.
protected abstract SyntaxNode? GetArgumentByPositionOrName(IInvocationOperation invocation, int index, string name, out bool isNamed);

// Verifies if a namespace has already been added to the usings/imports list.
protected abstract bool IsSystemNamespaceImported(IReadOnlyList<SyntaxNode> importList);

// Verifies if the user passed `0` as the 1st argument (`offset`) and `buffer.Length` as the 2nd argument (`count`),
// where `buffer` is the name of the variable passed as the 0th argument.
protected abstract bool IsPassingZeroAndBufferLength(SemanticModel model, SyntaxNode bufferValueNode, SyntaxNode offsetValueNode, SyntaxNode countValueNode);
Expand Down Expand Up @@ -152,7 +151,7 @@ private Task<Document> FixInvocationAsync(SemanticModel model, Document doc, Syn
SyntaxNode asMemoryInvocationNode = generator.InvocationExpression(
asMemoryExpressionNode,
namedStartNode.WithTriviaFrom(offsetNode),
namedLengthNode.WithTriviaFrom(countNode));
namedLengthNode.WithTriviaFrom(countNode)).WithAddImportsAnnotation().WithAdditionalAnnotations(s_asMemorySymbolAnnotation);

// Generate the new buffer argument, ensuring we include the buffer argument name if the user originally indicated one
replacedInvocationNode = GetNamedArgument(generator, asMemoryInvocationNode, isBufferNamed, "buffer")
Expand All @@ -175,14 +174,9 @@ private Task<Document> FixInvocationAsync(SemanticModel model, Document doc, Syn
}

SyntaxNode newInvocationExpression = generator.InvocationExpression(asyncMethodNode, nodeArguments).WithTriviaFrom(streamInstanceNode);

bool containsSystemImport = IsSystemNamespaceImported(generator.GetNamespaceImports(root));

// The invocation needs to be replaced before adding the import/using, it won't work the other way around
SyntaxNode newRoot = generator.ReplaceNode(root, invocation.Syntax, newInvocationExpression.WithTriviaFrom(invocation.Syntax));
SyntaxNode newRootWithImports = containsSystemImport ? newRoot : generator.AddNamespaceImports(newRoot, generator.NamespaceImportDeclaration(nameof(System)));

return Task.FromResult(doc.WithSyntaxRoot(newRootWithImports));
return Task.FromResult(doc.WithSyntaxRoot(newRoot));
}
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
Expand All @@ -23,11 +22,10 @@ public abstract class UseSpanBasedStringConcatFixer : CodeFixProvider
private protected const string AsSpanName = nameof(MemoryExtensions.AsSpan);
private protected const string AsSpanStartParameterName = "start";
private protected const string ToStringName = nameof(ToString);
private static readonly SyntaxAnnotation s_asSpanSymbolAnnotation = new("SymbolId", "System.MemoryExtensions");

private protected abstract SyntaxNode ReplaceInvocationMethodName(SyntaxGenerator generator, SyntaxNode invocationSyntax, string newName);

private protected abstract bool IsSystemNamespaceImported(Project project, IReadOnlyList<SyntaxNode> namespaceImports);

private protected abstract IOperation WalkDownBuiltInImplicitConversionOnConcatOperand(IOperation operand);

private protected abstract bool IsNamedArgument(IArgumentOperation argumentOperation);
Expand Down Expand Up @@ -110,13 +108,6 @@ async Task<Document> FixConcatOperationChain(CancellationToken cancellationToken

SyntaxNode newRoot = generator.ReplaceNode(root, concatExpressionSyntax, concatMethodInvocationSyntax);

// Import 'System' namespace if it's absent.
if (!IsSystemNamespaceImported(context.Document.Project, generator.GetNamespaceImports(newRoot)))
{
SyntaxNode systemNamespaceImport = generator.NamespaceImportDeclaration(nameof(System));
newRoot = generator.AddNamespaceImports(newRoot, systemNamespaceImport);
}

editor.ReplaceNode(root, newRoot);
return editor.GetChangedDocument();
}
Expand All @@ -140,7 +131,7 @@ private SyntaxNode ConvertOperandToArgument(in RequiredSymbols symbols, SyntaxGe
invocationSyntax = generator.ReplaceNode(invocationSyntax, namedStartIndexArgument.Syntax, renamedArgumentSyntax);
}

var asSpanInvocationSyntax = ReplaceInvocationMethodName(generator, invocationSyntax, AsSpanName);
var asSpanInvocationSyntax = ReplaceInvocationMethodName(generator, invocationSyntax, AsSpanName).WithAddImportsAnnotation().WithAdditionalAnnotations(s_asSpanSymbolAnnotation);
return generator.Argument(asSpanInvocationSyntax);
}
// Character literals become string literals.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System.Collections.Generic;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Testing;
using Test.Utilities;
using Xunit;

#pragma warning disable CA1305 // Specify IFormatProvider in string.Format
Expand Down Expand Up @@ -576,7 +577,7 @@ public static IEnumerable<object[]> CSharpInlineByteArrayTestData()
"(new byte[s.Length]).AsMemory(0, (int)s.Length), new CancellationToken()" };
}

[Fact]
[WindowsOnlyFact] // https://github.com/dotnet/roslyn/issues/65081
public Task CS_Fixer_Diagnostic_EnsureSystemNamespaceAutoAddedAsync()
{
string originalCode = @"
Expand All @@ -594,10 +595,9 @@ public async void M()
}
}";
string fixedCode = @"
using System;
using System.IO;
using System.Threading;
using System;
class C
{
public async void M()
Expand Down Expand Up @@ -890,7 +890,7 @@ public static IEnumerable<object[]> VisualBasicInlineByteArrayTestData()
@"(New Byte(s.Length - 1) {}).AsMemory(0, s.Length), New CancellationToken()" };
}

[Fact]
[WindowsOnlyFact] // https://github.com/dotnet/roslyn/issues/65081
public Task VB_Fixer_Diagnostic_EnsureSystemNamespaceAutoAddedAsync()
{
string originalCode = @"
Expand All @@ -906,10 +906,9 @@ End Sub
End Class
";
string fixedCode = @"
Imports System
Imports System.IO
Imports System.Threading
Imports System
Class C
Public Async Sub M()
Using s As FileStream = New FileStream(""path.txt"", FileMode.Create)
Expand Down Expand Up @@ -1211,4 +1210,4 @@ private DiagnosticResult GetVisualBasicResult(int startLine, int startColumn, in

#endregion
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System.Collections.Generic;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Testing;
using Test.Utilities;
using Xunit;

#pragma warning disable CA1305 // Specify IFormatProvider in string.Format
Expand Down Expand Up @@ -566,7 +567,7 @@ public static IEnumerable<object[]> CSharpInlinedByteArrayTestData()
"(new byte[]{ 0xBA, 0x5E, 0xBA, 0x11, 0xF0, 0x07, 0xBA, 0x11 }).AsMemory(0, 8), new CancellationToken()" };
}

[Fact]
[WindowsOnlyFact] // https://github.com/dotnet/roslyn/issues/65081
public Task CS_Fixer_Diagnostic_EnsureSystemNamespaceAutoAddedAsync()
{
string originalCode = @"
Expand All @@ -584,10 +585,9 @@ public async void M()
}
}";
string fixedCode = @"
using System;
using System.IO;
using System.Threading;
using System;
class C
{
public async void M()
Expand Down Expand Up @@ -850,7 +850,7 @@ public static IEnumerable<object[]> VisualBasicInlinedByteArrayTestData()
@"(New Byte() {&HBA, &H5E, &HBA, &H11, &HF0, &H07, &HBA, &H11}).AsMemory(0, 8), New CancellationToken()" };
}

[Fact]
[WindowsOnlyFact] // https://github.com/dotnet/roslyn/issues/65081
public Task VB_Fixer_Diagnostic_EnsureSystemNamespaceAutoAddedAsync()
{
string originalCode = @"
Expand All @@ -866,10 +866,9 @@ End Sub
End Class
";
string fixedCode = @"
Imports System
Imports System.IO
Imports System.Threading
Imports System
Class C
Public Async Sub M()
Using s As FileStream = File.Open(""path.txt"", FileMode.Open)
Expand Down Expand Up @@ -1139,4 +1138,4 @@ protected DiagnosticResult GetVisualBasicResult(int startLine, int startColumn,

#endregion
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -761,6 +761,31 @@ public Task WithNonStringNonCharLiteralOperands_NoDiagnostic_VBAsync(string expr
};
return test.RunAsync();
}

[Fact]
public Task TestSystemImportedFromGlobalUsing()
{
var test = new VerifyCS.Test
{
TestState =
{
Sources =
{
"global using System;", CSWithBody("var _ = [|foo + bar.Substring(1)|];"),
},
},
FixedState =
{
Sources =
{
"global using System;", CSWithBody("var _ = string.Concat(foo, bar.AsSpan(1));"),
},
},
ReferenceAssemblies = ReferenceAssemblies.Net.Net50,
LanguageVersion = CodeAnalysis.CSharp.LanguageVersion.CSharp10,
};
return test.RunAsync();
}
#endregion

#region Helpers
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
' Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

Imports System.Composition
Imports Microsoft.CodeAnalysis
Imports Microsoft.CodeAnalysis.CodeFixes
Imports Microsoft.CodeAnalysis.Editing
Expand All @@ -9,7 +10,7 @@ Imports Microsoft.NetCore.Analyzers.Runtime

Namespace Microsoft.NetCore.VisualBasic.Analyzers.Runtime

<ExportCodeFixProvider(LanguageNames.VisualBasic)>
<ExportCodeFixProvider(LanguageNames.VisualBasic), [Shared]>
Public NotInheritable Class BasicPreferStreamAsyncMemoryOverloadsFixer

Inherits PreferStreamAsyncMemoryOverloadsFixer
Expand Down Expand Up @@ -39,25 +40,6 @@ Namespace Microsoft.NetCore.VisualBasic.Analyzers.Runtime
Return Nothing
End Function

Protected Overrides Function IsSystemNamespaceImported(importList As IReadOnlyList(Of SyntaxNode)) As Boolean
For Each import As SyntaxNode In importList
Dim importsStatement = TryCast(import, ImportsStatementSyntax)
If importsStatement IsNot Nothing Then
For Each clause As ImportsClauseSyntax In importsStatement.ImportsClauses
Dim simpleClause = TryCast(clause, SimpleImportsClauseSyntax)
If simpleClause IsNot Nothing Then
Dim identifier = TryCast(simpleClause.Name, IdentifierNameSyntax)
If identifier IsNot Nothing AndAlso String.Equals(identifier.Identifier.Text, "System", StringComparison.OrdinalIgnoreCase) Then
Return True
End If
End If
Next
End If
Next

Return False
End Function

Protected Overrides Function IsPassingZeroAndBufferLength(model As SemanticModel, bufferValueNode As SyntaxNode, offsetValueNode As SyntaxNode, countValueNode As SyntaxNode) As Boolean
' First argument should be an identifier name node
Dim arg1 = TryCast(bufferValueNode, ArgumentSyntax)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ Imports Microsoft.CodeAnalysis
Imports Microsoft.CodeAnalysis.CodeFixes
Imports Microsoft.CodeAnalysis.Editing
Imports Microsoft.CodeAnalysis.Operations
Imports Microsoft.CodeAnalysis.VisualBasic
Imports Microsoft.CodeAnalysis.VisualBasic.Syntax
Imports Microsoft.NetCore.Analyzers.Runtime

Expand All @@ -22,35 +21,6 @@ Namespace Microsoft.NetCore.VisualBasic.Analyzers.Runtime
Return invocationSyntax.ReplaceNode(oldNameSyntax, newNameSyntax)
End Function

Private Protected Overrides Function IsSystemNamespaceImported(project As Project, namespaceImports As IReadOnlyList(Of SyntaxNode)) As Boolean

Dim options = DirectCast(project.CompilationOptions, VisualBasicCompilationOptions)
If options.GlobalImports.Any(Function(x) String.Compare(x.Name, NameOf(System), StringComparison.OrdinalIgnoreCase) = 0) Then
Return True
End If

For Each node As SyntaxNode In namespaceImports
Dim importsStatement = TryCast(node, ImportsStatementSyntax)
If importsStatement Is Nothing Then
Continue For
End If

For Each importsClause As ImportsClauseSyntax In importsStatement.ImportsClauses
Dim simpleClause = TryCast(importsClause, SimpleImportsClauseSyntax)
Dim identifierName = TryCast(simpleClause?.Name, IdentifierNameSyntax)
If identifierName Is Nothing Then
Continue For
End If

If identifierName.Identifier.ValueText = NameOf(System) Then
Return True
End If
Next
Next

Return False
End Function

Private Protected Overrides Function WalkDownBuiltInImplicitConversionOnConcatOperand(operand As IOperation) As IOperation

Return UseSpanBasedStringConcat.BasicWalkDownBuiltInImplicitConversionOnConcatOperand(operand)
Expand Down

0 comments on commit f983f0e

Please sign in to comment.