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

UpgradeProject code fixer for C# #17435

Merged
merged 25 commits into from
Mar 15, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
b61f8f6
UpgradeProject code fixer for C#
jcouv Feb 24, 2017
5331648
Removing un-necessary InternalVisibleTo I had added and using linked
jcouv Feb 27, 2017
16bee18
Introduce language service abstraction
jcouv Feb 28, 2017
6347aa9
Remove un-necessary asynchrony
jcouv Feb 28, 2017
a8cde3e
Refining IParseOptionsService interface
jcouv Feb 28, 2017
5448bf4
PR feedback (2)
jcouv Feb 28, 2017
685fafd
Use SpecifiedLanguageVersion, rename API to TryParse and extract version
jcouv Mar 1, 2017
d86922f
PR feedback (3)
jcouv Mar 2, 2017
7e895df
Updating test
jcouv Mar 2, 2017
280689b
Merging implementations of TryParseLanguageVersion
jcouv Mar 3, 2017
f3461b0
Use published VSLangeProj2 package
jcouv Mar 7, 2017
7752d56
Fixing bad merge
Mar 9, 2017
ab8cbf9
Fix nuget downgrade warning
jcouv Mar 9, 2017
7b9d347
Updating test following rebase
jcouv Mar 9, 2017
97a9af4
PR feedback
jcouv Mar 9, 2017
e913a27
Add test for fix-all scenario
jcouv Mar 10, 2017
cb0dae3
Add test for code action titles
jcouv Mar 10, 2017
1da8634
Check version too before offering to 'fix all'
jcouv Mar 10, 2017
73d3709
Moving CanApplyParseOptionChange logic down into VS Workspace layer
jcouv Mar 10, 2017
ead449f
Adding tests for VB GetRequiredLanguageVersion
jcouv Mar 10, 2017
2c952ac
Implement corresponding VB APIs
jcouv Mar 11, 2017
5820b9c
Re-organizing compiler code from discussion with Aleksey and Chuck
jcouv Mar 13, 2017
2d48bdf
Tweaks
jcouv Mar 13, 2017
2c5cf56
Fix spacing, static reference on instance instead of type, throw if n…
jcouv Mar 13, 2017
a6cdef3
Reverting files that only differnt in spaces
jcouv Mar 14, 2017
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
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2079,7 +2079,7 @@ internal static void CheckFeatureAvailability(SyntaxNode syntax, MessageID featu
LanguageVersion requiredVersion = feature.RequiredVersion();
if (requiredVersion > availableVersion)
{
diagnostics.Add(availableVersion.GetErrorCode(), location, feature.Localize(), requiredVersion.Localize());
diagnostics.Add(availableVersion.GetErrorCode(), location, feature.Localize(), new CSharpRequiredLanguageVersion(requiredVersion));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -805,7 +805,7 @@ internal sealed override CommandLineArguments CommonParse(IEnumerable<string> ar
{
AddDiagnostic(diagnostics, ErrorCode.ERR_SwitchNeedsString, MessageID.IDS_Text.Localize(), "/langversion:");
}
else if (!TryParseLanguageVersion(value, out languageVersion))
else if (!value.TryParse(out languageVersion))
{
AddDiagnostic(diagnostics, ErrorCode.ERR_BadCompatMode, value);
}
Expand Down Expand Up @@ -1760,56 +1760,6 @@ internal static ResourceDescription ParseResourceDescription(
return new ResourceDescription(resourceName, fileName, dataProvider, isPublic, embedded, checkArgs: false);
}

private static bool TryParseLanguageVersion(string str, out LanguageVersion version)
{
if (str == null)
{
version = LanguageVersion.Default;
return true;
}

switch (str.ToLowerInvariant())
{
case "iso-1":
version = LanguageVersion.CSharp1;
return true;

case "iso-2":
version = LanguageVersion.CSharp2;
return true;

case "7":
version = LanguageVersion.CSharp7;
return true;

case "default":
version = LanguageVersion.Default;
return true;

case "latest":
version = LanguageVersion.Latest;
return true;

default:
// We are likely to introduce minor version numbers after C# 7, thus breaking the
// one-to-one correspondence between the integers and the corresponding
// LanguageVersion enum values. But for compatibility we continue to accept any
// integral value parsed by int.TryParse for its corresponding LanguageVersion enum
// value for language version C# 6 and earlier (e.g. leading zeros are allowed)
int versionNumber;
if (int.TryParse(str, NumberStyles.None, CultureInfo.InvariantCulture, out versionNumber) &&
versionNumber <= 6 &&
((LanguageVersion)versionNumber).IsValid())
{
version = (LanguageVersion)versionNumber;
return true;
}

version = LanguageVersion.Default;
return false;
}
}

private static IEnumerable<string> ParseWarnings(string value)
{
value = value.Unquote();
Expand Down
136 changes: 117 additions & 19 deletions src/Compilers/CSharp/Portable/LanguageVersion.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// 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.Globalization;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.CSharp
Expand Down Expand Up @@ -74,30 +75,13 @@ public enum LanguageVersion
Latest = int.MaxValue,
}

internal static partial class LanguageVersionExtensions
internal static class LanguageVersionExtensionsInternal
{
internal static LanguageVersion MapSpecifiedToEffectiveVersion(this LanguageVersion version)
{
switch (version)
{
case LanguageVersion.Latest:
case LanguageVersion.Default:
return LanguageVersion.CSharp7;
default:
return version;
}
}

internal static bool IsValid(this LanguageVersion value)
{
return value >= LanguageVersion.CSharp1 && value <= LanguageVersion.CSharp7;
}

internal static object Localize(this LanguageVersion value)
{
return (int)value;
}

internal static ErrorCode GetErrorCode(this LanguageVersion version)
{
switch (version)
Expand All @@ -121,4 +105,118 @@ internal static ErrorCode GetErrorCode(this LanguageVersion version)
}
}
}
}

internal class CSharpRequiredLanguageVersion : RequiredLanguageVersion
{
internal LanguageVersion Version { get; }

internal CSharpRequiredLanguageVersion(LanguageVersion version)
{
Version = version;
}

public override string ToString() => Version.ToDisplayString();
}

public static class LanguageVersionFacts
{
/// <summary>
/// Displays the version number in the format expected on the command-line (/langver flag).
/// For instance, "6", "7", "7.1", "latest".
/// </summary>
public static string ToDisplayString(this LanguageVersion version)
{
switch (version)
{
case LanguageVersion.CSharp1:
return "1";
case LanguageVersion.CSharp2:
return "2";
case LanguageVersion.CSharp3:
return "3";
case LanguageVersion.CSharp4:
return "4";
case LanguageVersion.CSharp5:
return "5";
case LanguageVersion.CSharp6:
return "6";
case LanguageVersion.CSharp7:
return "7";
Copy link
Member

Choose a reason for hiding this comment

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

Support for 7.1 is happening in the other PR? I think so, just trying to wrap my head around it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, 7.1 will come separately. I'm still trying to work out some logistics (should the 7.1 change be made in a branch? which one? etc).
Also, adding 7.1 separately will be a good dry-run to verify for the checklist.

case LanguageVersion.Default:
return "default";
case LanguageVersion.Latest:
return "latest";
default:
throw ExceptionUtilities.UnexpectedValue(version);
}
}

/// <summary>
/// Parse a LanguageVersion from a string input, as the command-line compiler does.
/// </summary>
public static bool TryParse(this string version, out LanguageVersion result)
{
if (version == null)
{
result = LanguageVersion.Default;
return true;
}

switch (version.ToLowerInvariant())
{
case "iso-1":
result = LanguageVersion.CSharp1;
return true;

case "iso-2":
result = LanguageVersion.CSharp2;
return true;

case "7":
result = LanguageVersion.CSharp7;
return true;

case "default":
result = LanguageVersion.Default;
return true;

case "latest":
result = LanguageVersion.Latest;
return true;

default:
// We are likely to introduce minor version numbers after C# 7, thus breaking the
// one-to-one correspondence between the integers and the corresponding
// LanguageVersion enum values. But for compatibility we continue to accept any
// integral value parsed by int.TryParse for its corresponding LanguageVersion enum
// value for language version C# 6 and earlier (e.g. leading zeros are allowed)
int versionNumber;
if (int.TryParse(version, NumberStyles.None, CultureInfo.InvariantCulture, out versionNumber) &&
versionNumber <= 6 &&
((LanguageVersion)versionNumber).IsValid())
{
result = (LanguageVersion)versionNumber;
return true;
}

result = LanguageVersion.Default;
return false;
}
}

/// <summary>
/// Map a language version (such as Default, Latest, or CSharpN) to a specific version (CSharpM).
/// </summary>
public static LanguageVersion MapSpecifiedToEffectiveVersion(this LanguageVersion version)
{
switch (version)
{
case LanguageVersion.Latest:
case LanguageVersion.Default:
return LanguageVersion.CSharp7;
default:
return version;
}
}
}
}
3 changes: 2 additions & 1 deletion src/Compilers/CSharp/Portable/Parser/Lexer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -916,7 +916,8 @@ private void CheckFeatureAvailability(MessageID feature)
var requiredVersion = feature.RequiredVersion();
if (availableVersion >= requiredVersion) return;
var featureName = feature.Localize();
this.AddError(availableVersion.GetErrorCode(), featureName, requiredVersion.Localize());

this.AddError(availableVersion.GetErrorCode(), featureName, new CSharpRequiredLanguageVersion(requiredVersion));
}

private bool ScanInteger()
Expand Down
5 changes: 3 additions & 2 deletions src/Compilers/CSharp/Portable/Parser/SyntaxParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1074,11 +1074,12 @@ protected TNode CheckFeatureAvailability<TNode>(TNode node, MessageID feature, b

if (forceWarning)
{
SyntaxDiagnosticInfo rawInfo = new SyntaxDiagnosticInfo(availableVersion.GetErrorCode(), featureName, requiredVersion.Localize());
SyntaxDiagnosticInfo rawInfo = new SyntaxDiagnosticInfo(availableVersion.GetErrorCode(), featureName,
new CSharpRequiredLanguageVersion(requiredVersion));
return this.AddError(node, ErrorCode.WRN_ErrorOverride, rawInfo, rawInfo.Code);
}

return this.AddError(node, availableVersion.GetErrorCode(), featureName, requiredVersion.Localize());
return this.AddError(node, availableVersion.GetErrorCode(), featureName, new CSharpRequiredLanguageVersion(requiredVersion));
}
}

Expand Down
4 changes: 4 additions & 0 deletions src/Compilers/CSharp/Portable/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ Microsoft.CodeAnalysis.CSharp.Conversion.IsTupleLiteralConversion.get -> bool
Microsoft.CodeAnalysis.CSharp.LanguageVersion.CSharp7 = 7 -> Microsoft.CodeAnalysis.CSharp.LanguageVersion
Microsoft.CodeAnalysis.CSharp.LanguageVersion.Default = 0 -> Microsoft.CodeAnalysis.CSharp.LanguageVersion
Microsoft.CodeAnalysis.CSharp.LanguageVersion.Latest = 2147483647 -> Microsoft.CodeAnalysis.CSharp.LanguageVersion
Microsoft.CodeAnalysis.CSharp.LanguageVersionFacts
Microsoft.CodeAnalysis.CSharp.Syntax.AccessorDeclarationSyntax.ExpressionBody.get -> Microsoft.CodeAnalysis.CSharp.Syntax.ArrowExpressionClauseSyntax
Microsoft.CodeAnalysis.CSharp.Syntax.AccessorDeclarationSyntax.Update(Microsoft.CodeAnalysis.SyntaxList<Microsoft.CodeAnalysis.CSharp.Syntax.AttributeListSyntax> attributeLists, Microsoft.CodeAnalysis.SyntaxTokenList modifiers, Microsoft.CodeAnalysis.SyntaxToken keyword, Microsoft.CodeAnalysis.CSharp.Syntax.BlockSyntax body, Microsoft.CodeAnalysis.CSharp.Syntax.ArrowExpressionClauseSyntax expressionBody, Microsoft.CodeAnalysis.SyntaxToken semicolonToken) -> Microsoft.CodeAnalysis.CSharp.Syntax.AccessorDeclarationSyntax
Microsoft.CodeAnalysis.CSharp.Syntax.AccessorDeclarationSyntax.WithExpressionBody(Microsoft.CodeAnalysis.CSharp.Syntax.ArrowExpressionClauseSyntax expressionBody) -> Microsoft.CodeAnalysis.CSharp.Syntax.AccessorDeclarationSyntax
Expand Down Expand Up @@ -260,6 +261,9 @@ static Microsoft.CodeAnalysis.CSharp.CSharpExtensions.GetDeclaredSymbol(this Mic
static Microsoft.CodeAnalysis.CSharp.CSharpExtensions.GetDeclaredSymbol(this Microsoft.CodeAnalysis.SemanticModel semanticModel, Microsoft.CodeAnalysis.CSharp.Syntax.TupleElementSyntax declarationSyntax, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> Microsoft.CodeAnalysis.ISymbol
static Microsoft.CodeAnalysis.CSharp.CSharpExtensions.GetDeclaredSymbol(this Microsoft.CodeAnalysis.SemanticModel semanticModel, Microsoft.CodeAnalysis.CSharp.Syntax.TupleExpressionSyntax declaratorSyntax, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> Microsoft.CodeAnalysis.INamedTypeSymbol
static Microsoft.CodeAnalysis.CSharp.CSharpExtensions.GetForEachStatementInfo(this Microsoft.CodeAnalysis.SemanticModel semanticModel, Microsoft.CodeAnalysis.CSharp.Syntax.CommonForEachStatementSyntax forEachStatement) -> Microsoft.CodeAnalysis.CSharp.ForEachStatementInfo
static Microsoft.CodeAnalysis.CSharp.LanguageVersionFacts.MapSpecifiedToEffectiveVersion(this Microsoft.CodeAnalysis.CSharp.LanguageVersion version) -> Microsoft.CodeAnalysis.CSharp.LanguageVersion
static Microsoft.CodeAnalysis.CSharp.LanguageVersionFacts.ToDisplayString(this Microsoft.CodeAnalysis.CSharp.LanguageVersion version) -> string
static Microsoft.CodeAnalysis.CSharp.LanguageVersionFacts.TryParse(this string version, out Microsoft.CodeAnalysis.CSharp.LanguageVersion result) -> bool
static Microsoft.CodeAnalysis.CSharp.SyntaxFactory.AccessorDeclaration(Microsoft.CodeAnalysis.CSharp.SyntaxKind kind) -> Microsoft.CodeAnalysis.CSharp.Syntax.AccessorDeclarationSyntax
static Microsoft.CodeAnalysis.CSharp.SyntaxFactory.AccessorDeclaration(Microsoft.CodeAnalysis.CSharp.SyntaxKind kind, Microsoft.CodeAnalysis.CSharp.Syntax.BlockSyntax body) -> Microsoft.CodeAnalysis.CSharp.Syntax.AccessorDeclarationSyntax
static Microsoft.CodeAnalysis.CSharp.SyntaxFactory.AccessorDeclaration(Microsoft.CodeAnalysis.CSharp.SyntaxKind kind, Microsoft.CodeAnalysis.SyntaxList<Microsoft.CodeAnalysis.CSharp.Syntax.AttributeListSyntax> attributeLists, Microsoft.CodeAnalysis.SyntaxTokenList modifiers, Microsoft.CodeAnalysis.CSharp.Syntax.ArrowExpressionClauseSyntax expressionBody) -> Microsoft.CodeAnalysis.CSharp.Syntax.AccessorDeclarationSyntax
Expand Down
59 changes: 57 additions & 2 deletions src/Compilers/CSharp/Test/CommandLine/CommandLineTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1325,13 +1325,68 @@ public void LangVersion()
}

[Fact]
public void RememberToUpdateDiagnosticsWhenUpdatingLangVersion()
public void LanguageVersionAdded_Canary()
{
// When new language versions are added, this test will fail. Remember to update the diagnostics message (ERR_BadCompatMode).
// When a new version is added, this test will break. This list must be checked:
// - update the command-line error for bad /langver flag (<see cref="ErrorCode.ERR_BadCompatMode"/>)
// - update the "UpgradeProject" codefixer
// - update the IDE drop-down for selecting Language Version
// - don't fix the canary test until you update all the tests that include it
Assert.Equal(LanguageVersion.CSharp7, LanguageVersion.Latest.MapSpecifiedToEffectiveVersion());
Assert.Equal(LanguageVersion.CSharp7, LanguageVersion.Default.MapSpecifiedToEffectiveVersion());
}

[Fact]
public void LanguageVersion_DisplayString()
{
AssertEx.SetEqual(new[] { "default", "1", "2", "3", "4", "5", "6", "7", "latest" },
Enum.GetValues(typeof(LanguageVersion)).Cast<LanguageVersion>().Select(v => v.ToDisplayString()));
// For minor versions, the format should be "x.y", such as "7.1"
}

[Fact]
public void LanguageVersion_MapSpecifiedToEffectiveVersion()
{
Assert.Equal(LanguageVersion.CSharp1, LanguageVersion.CSharp1.MapSpecifiedToEffectiveVersion());
Copy link
Member

Choose a reason for hiding this comment

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

xunit as "theories" which let you run the same test with a bunch of bits of data like this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the suggestion. I gave it a try, but I didn't feel it fit that well.
One problem is that Default and Latest are irregularities, but it doesn't seem worth splitting the test or having two parameters. I'm tempted to leave as-is (seems simpler).
And there is also the canary that I want to include but not invoke many times.

Copy link
Member

Choose a reason for hiding this comment

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

Not sure I understand this, especially when you used a theory for the next test. 😄

Assert.Equal(LanguageVersion.CSharp2, LanguageVersion.CSharp2.MapSpecifiedToEffectiveVersion());
Assert.Equal(LanguageVersion.CSharp3, LanguageVersion.CSharp3.MapSpecifiedToEffectiveVersion());
Assert.Equal(LanguageVersion.CSharp4, LanguageVersion.CSharp4.MapSpecifiedToEffectiveVersion());
Assert.Equal(LanguageVersion.CSharp5, LanguageVersion.CSharp5.MapSpecifiedToEffectiveVersion());
Assert.Equal(LanguageVersion.CSharp6, LanguageVersion.CSharp6.MapSpecifiedToEffectiveVersion());
Assert.Equal(LanguageVersion.CSharp7, LanguageVersion.CSharp7.MapSpecifiedToEffectiveVersion());
Assert.Equal(LanguageVersion.CSharp7, LanguageVersion.Default.MapSpecifiedToEffectiveVersion());
Assert.Equal(LanguageVersion.CSharp7, LanguageVersion.Latest.MapSpecifiedToEffectiveVersion());

// The canary check is a reminder that this test needs to be updated when a language version is added
LanguageVersionAdded_Canary();
}

[Theory,
InlineData("iso-1", true, LanguageVersion.CSharp1),
InlineData("ISO-1", true, LanguageVersion.CSharp1),
InlineData("iso-2", true, LanguageVersion.CSharp2),
InlineData("1", true, LanguageVersion.CSharp1),
InlineData("2", true, LanguageVersion.CSharp2),
InlineData("3", true, LanguageVersion.CSharp3),
InlineData("4", true, LanguageVersion.CSharp4),
InlineData("5", true, LanguageVersion.CSharp5),
InlineData("05", true, LanguageVersion.CSharp5),
InlineData("6", true, LanguageVersion.CSharp6),
InlineData("7", true, LanguageVersion.CSharp7),
InlineData("07", false, LanguageVersion.Default),
InlineData("default", true, LanguageVersion.Default),
InlineData("latest", true, LanguageVersion.Latest),
InlineData(null, true, LanguageVersion.Default),
InlineData("bad", false, LanguageVersion.Default)]
public void LanguageVersion_TryParseDisplayString(string input, bool success, LanguageVersion expected)
{
Assert.Equal(success, input.TryParse(out var version));
Assert.Equal(expected, version);

// The canary check is a reminder that this test needs to be updated when a language version is added
LanguageVersionAdded_Canary();
}

[Fact]
[WorkItem(546961, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/546961")]
public void Define()
Expand Down
4 changes: 4 additions & 0 deletions src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenTupleTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9219,6 +9219,10 @@ static void Main()
// CS0151ERR_IntegralTypeValueExpected}
Diagnostic(ErrorCode.ERR_InvalidMemberDecl, "}").WithArguments("}").WithLocation(8, 36)
);

Assert.Equal("7", Compilation.GetRequiredLanguageVersion(comp.GetDiagnostics()[0]));
Assert.Null(Compilation.GetRequiredLanguageVersion(comp.GetDiagnostics()[2]));
Assert.Throws<ArgumentNullException>(() => Compilation.GetRequiredLanguageVersion(null));
}

[Fact]
Expand Down
Loading