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

Add style option (and UI) for preferring file scoped namespaces #55043

Merged
merged 50 commits into from
Jul 26, 2021
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
43a164b
Add style option (and UI) for preferring file scoped namespaces
CyrusNajmabadi Jul 21, 2021
3a1c6fb
Scaffolding
CyrusNajmabadi Jul 22, 2021
f8b8854
Working
CyrusNajmabadi Jul 22, 2021
4e04d3a
Whitespace
CyrusNajmabadi Jul 22, 2021
e1ca37a
working
CyrusNajmabadi Jul 22, 2021
da4a837
Fix check
CyrusNajmabadi Jul 22, 2021
fe29b72
Update src/Features/CSharp/Portable/ConvertNamespace/ConvertNamespace…
CyrusNajmabadi Jul 22, 2021
89f76f3
Update src/Features/CSharp/Portable/ConvertNamespace/ConvertNamespace…
CyrusNajmabadi Jul 22, 2021
ec9a8c1
Simplify
CyrusNajmabadi Jul 22, 2021
1cdf32d
Merge branch 'fileScopedNamespaceStyle' of https://github.com/CyrusNa…
CyrusNajmabadi Jul 22, 2021
6143064
Simplify
CyrusNajmabadi Jul 22, 2021
eab3b8f
Fix
CyrusNajmabadi Jul 22, 2021
ce024a7
Merge remote-tracking branch 'upstream/main' into fileScopedNamespace…
CyrusNajmabadi Jul 22, 2021
92e56ae
Add name
CyrusNajmabadi Jul 22, 2021
69eaa73
Code gen file scoped namespaces
CyrusNajmabadi Jul 22, 2021
275a415
Add test
CyrusNajmabadi Jul 22, 2021
cac2b64
Add metadata as source test
CyrusNajmabadi Jul 22, 2021
ac76a27
Add tests
CyrusNajmabadi Jul 22, 2021
4ddf9be
Add tests
CyrusNajmabadi Jul 22, 2021
1c48e49
Ad dtests
CyrusNajmabadi Jul 23, 2021
23594c9
use equiv key
CyrusNajmabadi Jul 23, 2021
8283983
Move to an enum
CyrusNajmabadi Jul 23, 2021
80f944b
Merge remote-tracking branch 'upstream/main' into fileScopedNamespace…
CyrusNajmabadi Jul 23, 2021
4c7b9db
switch to an enum
CyrusNajmabadi Jul 23, 2021
6f78817
Add tests
CyrusNajmabadi Jul 23, 2021
95b9cd5
Update formatter
CyrusNajmabadi Jul 23, 2021
08ec0a7
tests
CyrusNajmabadi Jul 23, 2021
10cecf5
Tests
CyrusNajmabadi Jul 23, 2021
d6e325c
Add tests
CyrusNajmabadi Jul 23, 2021
fb592a0
tests
CyrusNajmabadi Jul 23, 2021
7291263
Tests
CyrusNajmabadi Jul 23, 2021
3a6d042
add tests
CyrusNajmabadi Jul 23, 2021
a530117
Merge remote-tracking branch 'upstream/main' into fileScopedNamespace…
CyrusNajmabadi Jul 24, 2021
594d1be
Fix test
CyrusNajmabadi Jul 24, 2021
1656aa7
Before split
CyrusNajmabadi Jul 24, 2021
fe53780
Update tests
CyrusNajmabadi Jul 24, 2021
1186364
Tests
CyrusNajmabadi Jul 25, 2021
a0e35ea
Adjust test baseline for `CSharpAddMissingUsingsOnPaste.VerifyDisable…
jcouv Jul 25, 2021
4a09306
Merge remote-tracking branch 'upstream/devjcouvintegration-test' into…
CyrusNajmabadi Jul 25, 2021
13533a0
Add name
CyrusNajmabadi Jul 25, 2021
a72e35e
Update tests
CyrusNajmabadi Jul 25, 2021
34541ed
Share code
CyrusNajmabadi Jul 26, 2021
91cd171
Simplify
CyrusNajmabadi Jul 26, 2021
3cc0cc9
NRT
CyrusNajmabadi Jul 26, 2021
403ac97
tests
CyrusNajmabadi Jul 26, 2021
4411dab
Fix formatting
CyrusNajmabadi Jul 26, 2021
3097152
Fix tests
CyrusNajmabadi Jul 26, 2021
fd48f8e
Move to analyzer lib
CyrusNajmabadi Jul 26, 2021
45e7a90
Update automation object
CyrusNajmabadi Jul 26, 2021
42261eb
Update src/Analyzers/CSharp/Analyzers/CSharpAnalyzers.projitems
CyrusNajmabadi Jul 26, 2021
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 @@ -50,13 +50,13 @@ protected static DiagnosticDescriptor CreateDescriptorWithId(
string id,
EnforceOnBuild enforceOnBuild,
LocalizableString title,
LocalizableString messageFormat,
LocalizableString? messageFormat = null,
bool isUnnecessary = false,
bool isConfigurable = true,
LocalizableString? description = null)
#pragma warning disable RS0030 // Do not used banned APIs
=> new(
id, title, messageFormat,
id, title, messageFormat ?? title,
DiagnosticCategory.Style,
DiagnosticSeverity.Hidden,
isEnabledByDefault: true,
Expand Down
2 changes: 2 additions & 0 deletions src/Analyzers/Core/Analyzers/EnforceOnBuildValues.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ internal static class EnforceOnBuildValues
public const EnforceOnBuild InvalidSuppressMessageAttribute = /*IDE0076*/ EnforceOnBuild.HighlyRecommended;
public const EnforceOnBuild LegacyFormatSuppressMessageAttribute = /*IDE0077*/ EnforceOnBuild.HighlyRecommended;
public const EnforceOnBuild RemoveConfusingSuppressionForIsExpression = /*IDE0080*/ EnforceOnBuild.HighlyRecommended;
public const EnforceOnBuild UseRegularNamespace = /*IDE0160*/ EnforceOnBuild.HighlyRecommended;
public const EnforceOnBuild UseFileScopedNamespace = /*IDE0161*/ EnforceOnBuild.HighlyRecommended;

/* EnforceOnBuild.Recommended */
public const EnforceOnBuild UseThrowExpression = /*IDE0016*/ EnforceOnBuild.Recommended;
Expand Down
3 changes: 3 additions & 0 deletions src/Analyzers/Core/Analyzers/IDEDiagnosticIds.cs
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,9 @@ internal static class IDEDiagnosticIds

public const string UseNullCheckOverTypeCheckDiagnosticId = "IDE0150";

public const string UseRegularNamespaceDiagnosticId = "IDE0160";
public const string UseFileScopedNamespaceDiagnosticId = "IDE0161";

// Analyzer error Ids
public const string AnalyzerChangedId = "IDE1001";
public const string AnalyzerDependencyConflictId = "IDE1002";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ internal static class PredefinedCodeFixProviderNames
public const string ChangeToYield = nameof(ChangeToYield);
public const string ConvertToAsync = nameof(ConvertToAsync);
public const string ConvertToIterator = nameof(ConvertToIterator);
public const string ConvertNamespace = nameof(ConvertNamespace);
public const string CorrectNextControlVariable = nameof(CorrectNextControlVariable);
public const string ConvertTypeOfToNameOf = nameof(ConvertTypeOfToNameOf);
public const string RemoveDocCommentNode = nameof(RemoveDocCommentNode);
Expand Down
8 changes: 8 additions & 0 deletions src/Features/CSharp/Portable/CSharpFeaturesResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -628,4 +628,12 @@
<data name="record_struct" xml:space="preserve">
<value>record struct</value>
</data>
<data name="Convert_to_file_scoped_namespace" xml:space="preserve">
<value>Convert to file-scoped namespace</value>
<comment>{Locked="namespace"} "namespace" is a C# keyword and should not be localized.</comment>
</data>
<data name="Convert_to_regular_namespace" xml:space="preserve">
<value>Convert to regular namespace</value>
<comment>{Locked="namespace"} "namespace" is a C# keyword and should not be localized.</comment>
</data>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Collections.Immutable;
using System.Composition;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Editing;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.Shared.Extensions;

namespace Microsoft.CodeAnalysis.CSharp.ConvertNamespace
{
[ExportCodeFixProvider(LanguageNames.CSharp, Name = PredefinedCodeFixProviderNames.ConvertNamespace), Shared]
internal class ConvertNamespaceCodeFixProvider : SyntaxEditorBasedCodeFixProvider
{
[ImportingConstructor]
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
public ConvertNamespaceCodeFixProvider()
{
}

internal override CodeFixCategory CodeFixCategory => CodeFixCategory.CodeStyle;
public override ImmutableArray<string> FixableDiagnosticIds
=> ImmutableArray.Create(IDEDiagnosticIds.UseRegularNamespaceDiagnosticId, IDEDiagnosticIds.UseFileScopedNamespaceDiagnosticId);

public override Task RegisterCodeFixesAsync(CodeFixContext context)
{
var diagnostic = context.Diagnostics.First();

var title = diagnostic.Id == IDEDiagnosticIds.UseFileScopedNamespaceDiagnosticId
? CSharpFeaturesResources.Convert_to_file_scoped_namespace
: CSharpFeaturesResources.Convert_to_regular_namespace;

context.RegisterCodeFix(
new MyCodeAction(title, c => FixAsync(context.Document, diagnostic, c)),
context.Diagnostics);

return Task.CompletedTask;
}

protected override Task FixAllAsync(
Document document, ImmutableArray<Diagnostic> diagnostics,
SyntaxEditor editor, CancellationToken cancellationToken)
{
var diagnostic = diagnostics.First();

var namespaceDecl = (BaseNamespaceDeclarationSyntax)diagnostic.Location.FindNode(cancellationToken);
var converted = ConvertNamespaceHelper.Convert(namespaceDecl);

editor.ReplaceNode(namespaceDecl, converted);
return Task.CompletedTask;
}

private class MyCodeAction : CodeAction.DocumentChangeAction
{
public MyCodeAction(string title, Func<CancellationToken, Task<Document>> createChangedDocument)
: base(title, createChangedDocument, title)
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
{
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Composition;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CodeRefactorings;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.CSharp.ConvertNamespace
{
[ExportCodeRefactoringProvider(LanguageNames.CSharp), Shared]
internal class ConvertNamespaceCodeRefactoringProvider : CodeRefactoringProvider
{
[ImportingConstructor]
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
public ConvertNamespaceCodeRefactoringProvider()
{
}

public override async Task ComputeRefactoringsAsync(CodeRefactoringContext context)
{
var (document, span, cancellationToken) = context;
if (!span.IsEmpty)
return;

var position = span.Start;
var root = (CompilationUnitSyntax)await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
var token = root.FindToken(position);
var namespaceDecl = token.GetAncestor<BaseNamespaceDeclarationSyntax>();
if (namespaceDecl == null)
return;

if (!IsValidPosition(namespaceDecl, position))
return;

var optionSet = await document.GetOptionsAsync(cancellationToken).ConfigureAwait(false);
var title =
ConvertNamespaceHelper.CanOfferUseRegular(optionSet, namespaceDecl, forAnalyzer: false) ? CSharpFeaturesResources.Convert_to_regular_namespace :
ConvertNamespaceHelper.CanOfferUseFileScoped(optionSet, root, namespaceDecl, forAnalyzer: false) ? CSharpFeaturesResources.Convert_to_file_scoped_namespace : null;
if (title == null)
return;

context.RegisterRefactoring(new MyCodeAction(
title, c => ConvertNamespaceHelper.ConvertAsync(document, namespaceDecl, c)));
}

private static bool IsValidPosition(BaseNamespaceDeclarationSyntax baseDeclaration, int position)
{
if (position < baseDeclaration.SpanStart)
return false;

if (baseDeclaration is FileScopedNamespaceDeclarationSyntax fileScopedNamespace)
return position <= fileScopedNamespace.SemicolonToken.Span.End;

if (baseDeclaration is NamespaceDeclarationSyntax namespaceDeclaration)
return position <= namespaceDeclaration.Name.Span.End;

throw ExceptionUtilities.UnexpectedValue(baseDeclaration.Kind());
}

private class MyCodeAction : CodeAction.DocumentChangeAction
{
public MyCodeAction(string title, Func<CancellationToken, Task<Document>> createChangedDocument)
: base(title, createChangedDocument, title)
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
{
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Collections.Immutable;
using Microsoft.CodeAnalysis.CodeStyle;
using Microsoft.CodeAnalysis.CSharp.CodeStyle;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.Text;

namespace Microsoft.CodeAnalysis.CSharp.ConvertNamespace
{
[DiagnosticAnalyzer(LanguageNames.CSharp)]
internal class ConvertNamespaceDiagnosticAnalyzer : AbstractBuiltInCodeStyleDiagnosticAnalyzer
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
{
private static readonly DiagnosticDescriptor s_useRegularNamespaceDescriptor = CreateDescriptorWithId(
IDEDiagnosticIds.UseRegularNamespaceDiagnosticId,
EnforceOnBuildValues.UseRegularNamespace,
new LocalizableResourceString(nameof(CSharpFeaturesResources.Convert_to_regular_namespace), CSharpFeaturesResources.ResourceManager, typeof(CSharpFeaturesResources)));

private static readonly DiagnosticDescriptor s_useFileScopedNamespaceDescriptor = CreateDescriptorWithId(
IDEDiagnosticIds.UseFileScopedNamespaceDiagnosticId,
EnforceOnBuildValues.UseFileScopedNamespace,
new LocalizableResourceString(nameof(CSharpFeaturesResources.Convert_to_file_scoped_namespace), CSharpFeaturesResources.ResourceManager, typeof(CSharpFeaturesResources)));

private static readonly ImmutableDictionary<DiagnosticDescriptor, ILanguageSpecificOption> s_descriptors =
ImmutableDictionary<DiagnosticDescriptor, ILanguageSpecificOption>.Empty
.Add(s_useRegularNamespaceDescriptor, CSharpCodeStyleOptions.PreferFileScopedNamespace)
.Add(s_useFileScopedNamespaceDescriptor, CSharpCodeStyleOptions.PreferFileScopedNamespace);

public ConvertNamespaceDiagnosticAnalyzer()
: base(s_descriptors, LanguageNames.CSharp)
{
}

public sealed override DiagnosticAnalyzerCategory GetAnalyzerCategory()
=> DiagnosticAnalyzerCategory.SemanticSpanAnalysis;

protected override void InitializeWorker(AnalysisContext context)
=> context.RegisterSyntaxNodeAction(AnalyzeNamespace, SyntaxKind.NamespaceDeclaration, SyntaxKind.FileScopedNamespaceDeclaration);

private void AnalyzeNamespace(SyntaxNodeAnalysisContext context)
{
var options = context.Options;
var namespaceDeclaration = (BaseNamespaceDeclarationSyntax)context.Node;
var syntaxTree = namespaceDeclaration.SyntaxTree;

var cancellationToken = context.CancellationToken;
var root = (CompilationUnitSyntax)syntaxTree.GetRoot(cancellationToken);

var optionSet = options.GetAnalyzerOptionSet(syntaxTree, cancellationToken);

var diagnostic = AnalyzeNamespace(optionSet, root, namespaceDeclaration);
if (diagnostic != null)
context.ReportDiagnostic(diagnostic);
}

private static Diagnostic? AnalyzeNamespace(OptionSet optionSet, CompilationUnitSyntax root, BaseNamespaceDeclarationSyntax declaration)
{
var tree = declaration.SyntaxTree;
var preferFileScopedNamespace = optionSet.GetOption(CSharpCodeStyleOptions.PreferFileScopedNamespace);
var severity = preferFileScopedNamespace.Notification.Severity;

var descriptor =
ConvertNamespaceHelper.CanOfferUseRegular(optionSet, declaration, forAnalyzer: true) ? s_useRegularNamespaceDescriptor :
ConvertNamespaceHelper.CanOfferUseFileScoped(optionSet, root, declaration, forAnalyzer: true) ? s_useFileScopedNamespaceDescriptor : null;
if (descriptor == null)
return null;

// if the diagnostic is hidden, show it anywhere from the `namespace` keyword through the name.
// otherwise, if it's not hidden, just squiggle the name.
var diagnosticLocation = severity.WithDefaultSeverity(DiagnosticSeverity.Hidden) == ReportDiagnostic.Hidden
Copy link
Member

Choose a reason for hiding this comment

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

I'm worried the severity variable here only reflects the option = value:severity syntax but not reflecting the dotnet_diagnostic.ID.severity = severity syntax.

Copy link
Member Author

Choose a reason for hiding this comment

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

so this follows what we do with use-expression-body. i'm going to stick with this pattern under teh presumption that it's correct. if not, we shoudl fix it for all of these. i do not want to have multiple different patterns at the same time :)

Copy link
Member

Choose a reason for hiding this comment

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

Tagging @mavasani to confirm if there is a problem here in which case I'll file an issue.

? tree.GetLocation(TextSpan.FromBounds(declaration.SpanStart, declaration.Name.Span.End))
: declaration.Name.GetLocation();

return DiagnosticHelper.Create(
descriptor,
diagnosticLocation,
severity,
ImmutableArray.Create(declaration.GetLocation()),
ImmutableDictionary<string, string>.Empty);
}
}
}
Loading