Skip to content

Naming style for local functions #26165

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.

Sign up for GitHub

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

Merged
merged 24 commits into from
Jun 1, 2018
Merged

Naming style for local functions #26165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
3ead6de
SymbolKindOrTypeKind -> SymbolOrTypeOrMethodKind
Neme12 Apr 15, 2018
6cb3529
Adding local function symbol specification
Neme12 Apr 15, 2018
06009c9
Adding EditorConfig test
Neme12 Apr 15, 2018
4484ca7
Adding code fix tests
Neme12 Apr 15, 2018
62c426c
Adding analysis of local functions
Neme12 May 10, 2018
2dcfaf5
Adding VS integration
Neme12 Apr 15, 2018
9d6a970
Adding local functions to async naming EditorConfig test
Neme12 Apr 15, 2018
531c92a
Adding code fix tests for async methods & local functions
Neme12 Apr 15, 2018
294eb5c
Changing tests
Neme12 Apr 15, 2018
4bb5b9b
Fixing DeclarationNameCompletionProvider to account for local functions
Neme12 Apr 16, 2018
2d07f56
Undoing rename
Neme12 Apr 16, 2018
d379694
Adding tests for custom naming style to DeclarationNameCompletionProv…
Neme12 Apr 16, 2018
a13fed9
Adding upgrade path for naming preferences
Neme12 Apr 16, 2018
e828dae
Details
Neme12 Apr 17, 2018
c0ca8d7
Test for correct order of naming suggestions
Neme12 Apr 17, 2018
de35f13
Modifier exclusion for local vs local function declaration
Neme12 Apr 18, 2018
8304bf8
Fixing logic for name suggestions inside using & for (they can't be a…
Neme12 Apr 20, 2018
393802e
Name suggestions for an actual fully-typed local function
Neme12 Apr 22, 2018
014dadd
Adding test with a higher than latest serialization version
Neme12 May 11, 2018
990246e
Merge remote-tracking branch 'upstream/master' into localFunctionNami…
Neme12 May 28, 2018
3f00166
Fix after merge
Neme12 May 28, 2018
d9262c2
Adding new roaming location to prevent syncing the new serialization …
Neme12 May 28, 2018
352226e
Removing outdated comment & adding missing test case
Neme12 May 29, 2018
ed4f34c
Switching to form without brackets
Neme12 May 31, 2018
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
165 changes: 162 additions & 3 deletions ...tures/CSharpTest/Completion/CompletionProviders/DeclarationNameCompletionProviderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,22 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Immutable;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Completion;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Completion.Providers;
using Microsoft.CodeAnalysis.Diagnostics.Analyzers.NamingStyles;
using Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.Completion.CompletionProviders;
using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces;
using Microsoft.CodeAnalysis.NamingStyles;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.Simplification;
using Microsoft.CodeAnalysis.Test.Utilities;
using Roslyn.Test.Utilities;
using Xunit;
using static Microsoft.CodeAnalysis.Diagnostics.Analyzers.NamingStyles.SymbolSpecification;

namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.Completion.CompletionSetSources
{
Expand All @@ -32,8 +40,8 @@ public class MyClass
MyClass $$
}
";
await VerifyItemExistsAsync(markup, "MyClass", glyph: (int)Glyph.MethodPublic);
await VerifyItemExistsAsync(markup, "myClass", glyph: (int)Glyph.FieldPublic);
await VerifyItemExistsAsync(markup, "MyClass", glyph: (int)Glyph.PropertyPublic);
await VerifyItemExistsAsync(markup, "GetMyClass", glyph: (int)Glyph.MethodPublic);
}

Expand Down Expand Up @@ -544,15 +552,49 @@ void goo()
}

[Fact, Trait(Traits.Feature, Traits.Features.Completion)]
public async Task TestDescription()
public async Task TestCorrectOrder()
{
var markup = @"
public class MyClass
{
MyClass $$
}
";
await VerifyItemExistsAsync(markup, "MyClass", glyph: (int)Glyph.MethodPublic, expectedDescriptionOrNull: CSharpFeaturesResources.Suggested_name);
var items = await GetCompletionItemsAsync(markup, SourceCodeKind.Regular);
Assert.Equal(
new[] { "myClass", "my", "@class", "MyClass", "My", "Class", "GetMyClass", "GetMy", "GetClass" },
items.Select(item => item.DisplayText));
}

[Fact, Trait(Traits.Feature, Traits.Features.Completion)]
public async Task TestDescriptionInsideClass()
{
var markup = @"
public class MyClass
{
MyClass $$
}
";
await VerifyItemExistsAsync(markup, "myClass", glyph: (int)Glyph.FieldPublic, expectedDescriptionOrNull: CSharpFeaturesResources.Suggested_name);
await VerifyItemExistsAsync(markup, "MyClass", glyph: (int)Glyph.PropertyPublic, expectedDescriptionOrNull: CSharpFeaturesResources.Suggested_name);
await VerifyItemExistsAsync(markup, "GetMyClass", glyph: (int)Glyph.MethodPublic, expectedDescriptionOrNull: CSharpFeaturesResources.Suggested_name);
}

[Fact, Trait(Traits.Feature, Traits.Features.Completion)]
public async Task TestDescriptionInsideMethod()
{
var markup = @"
public class MyClass
{
void M()
{
MyClass $$
}
}
";
await VerifyItemExistsAsync(markup, "myClass", glyph: (int)Glyph.Local, expectedDescriptionOrNull: CSharpFeaturesResources.Suggested_name);
await VerifyItemIsAbsentAsync(markup, "MyClass");
await VerifyItemIsAbsentAsync(markup, "GetMyClass");
}

[WorkItem(20273, "https://github.com/dotnet/roslyn/issues/20273")]
Expand Down Expand Up @@ -1411,5 +1453,122 @@ public void Method()
";
await VerifyItemExistsAsync(markup, "nullables");
}

[Fact, Trait(Traits.Feature, Traits.Features.Completion)]
public async Task CustomNamingStyleInsideClass()
{
var workspace = WorkspaceFixture.GetWorkspace();
var originalOptions = workspace.Options;

try
{
workspace.Options = workspace.Options.WithChangedOption(
new OptionKey(SimplificationOptions.NamingPreferences, LanguageNames.CSharp),
NamesEndWithSuffixPreferences());

var markup = @"
class Configuration
{
Configuration $$
}
";
await VerifyItemExistsAsync(markup, "ConfigurationField", glyph: (int)Glyph.FieldPublic,
expectedDescriptionOrNull: CSharpFeaturesResources.Suggested_name);
await VerifyItemExistsAsync(markup, "ConfigurationProperty", glyph: (int)Glyph.PropertyPublic,
expectedDescriptionOrNull: CSharpFeaturesResources.Suggested_name);
await VerifyItemExistsAsync(markup, "ConfigurationMethod", glyph: (int)Glyph.MethodPublic,
expectedDescriptionOrNull: CSharpFeaturesResources.Suggested_name);
await VerifyItemIsAbsentAsync(markup, "ConfigurationLocal");
await VerifyItemIsAbsentAsync(markup, "ConfigurationLocalFunction");
}
finally
{
workspace.Options = originalOptions;
}
}

[Fact, Trait(Traits.Feature, Traits.Features.Completion)]
public async Task CustomNamingStyleInsideMethod()
{
var workspace = WorkspaceFixture.GetWorkspace();
var originalOptions = workspace.Options;

try
{
workspace.Options = workspace.Options.WithChangedOption(
new OptionKey(SimplificationOptions.NamingPreferences, LanguageNames.CSharp),
NamesEndWithSuffixPreferences());
Copy link
Member

@CyrusNajmabadi CyrusNajmabadi Apr 16, 2018

Choose a reason for hiding this comment

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

do these tests run in parallel? if so, this would be a problem. Or is parallel running opt-in?

Copy link
Contributor Author

@Neme12 Neme12 Apr 16, 2018

Choose a reason for hiding this comment

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

this is what the existing tests do

Copy link
Contributor Author

@Neme12 Neme12 Apr 16, 2018

Choose a reason for hiding this comment

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

I thought there was a new test class instance created for each test, isn't that the case?

Copy link
Contributor Author

@Neme12 Neme12 Apr 16, 2018

Choose a reason for hiding this comment

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

and yes, parallel running is disabled by default

Copy link
Member

@CyrusNajmabadi CyrusNajmabadi Apr 17, 2018

Choose a reason for hiding this comment

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

Great. Thanks!


var markup = @"
class Configuration
{
void M()
{
Configuration $$
}
}
";
await VerifyItemExistsAsync(markup, "ConfigurationLocal", glyph: (int)Glyph.Local,
expectedDescriptionOrNull: CSharpFeaturesResources.Suggested_name);
await VerifyItemExistsAsync(markup, "ConfigurationLocalFunction", glyph: (int)Glyph.MethodPublic,
expectedDescriptionOrNull: CSharpFeaturesResources.Suggested_name);
await VerifyItemIsAbsentAsync(markup, "ConfigurationField");
await VerifyItemIsAbsentAsync(markup, "ConfigurationMethod");
await VerifyItemIsAbsentAsync(markup, "ConfigurationProperty");
}
finally
{
workspace.Options = originalOptions;
}
}

private static NamingStylePreferences NamesEndWithSuffixPreferences()
{
var specificationStyles = new[]
{
SpecificationStyle(new SymbolKindOrTypeKind(SymbolKind.Field), "Field"),
SpecificationStyle(new SymbolKindOrTypeKind(SymbolKind.Property), "Property"),
SpecificationStyle(new SymbolKindOrTypeKind(MethodKind.Ordinary), "Method"),
SpecificationStyle(new SymbolKindOrTypeKind(SymbolKind.Local), "Local"),
SpecificationStyle(new SymbolKindOrTypeKind(MethodKind.LocalFunction), "LocalFunction"),
};

return new NamingStylePreferences(
specificationStyles.Select(t => t.specification).ToImmutableArray(),
specificationStyles.Select(t => t.style).ToImmutableArray(),
specificationStyles.Select(t => CreateRule(t.specification, t.style)).ToImmutableArray());

// Local functions

(SymbolSpecification specification, NamingStyle style) SpecificationStyle(SymbolKindOrTypeKind kind, string suffix)
{
var symbolSpecification = new SymbolSpecification(
id: null,
symbolSpecName: suffix,
ImmutableArray.Create(kind),
ImmutableArray<Accessibility>.Empty,
ImmutableArray<ModifierKind>.Empty);

var namingStyle = new NamingStyle(
Guid.NewGuid(),
name: suffix,
capitalizationScheme: Capitalization.PascalCase,
prefix: "",
suffix: suffix,
wordSeparator: "");

return (symbolSpecification, namingStyle);
}

SerializableNamingRule CreateRule(SymbolSpecification specification, NamingStyle style)
{
return new SerializableNamingRule()
{
SymbolSpecificationID = specification.ID,
NamingStyleID = style.ID,
EnforcementLevel = DiagnosticSeverity.Error
};
}
}
}
}
Loading