Skip to content

Commit

Permalink
(GH-1260) Fix issue in parameter emitter where reserved keywords as l…
Browse files Browse the repository at this point in the history
…egal parameter names in assembly are not being generated correctly.

This provides proper escaping of all reserved keywords where the reflected  ParameterInfo.Name property of the parameter matches a reserved keyword.  At runtime, the CLR reports the un-escaped parameter name (the @parameter escaping is a compile-time hint only)

Update usage in ParameterEmitter, add supporting unit/integration tests.
  • Loading branch information
kcamp authored and devlead committed Mar 7, 2017
1 parent 67642b8 commit 70ba5e1
Show file tree
Hide file tree
Showing 7 changed files with 232 additions and 1 deletion.
6 changes: 6 additions & 0 deletions src/Cake.Core.Tests/Data/MethodAliasGeneratorData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -199,5 +199,11 @@ public static void NonGeneric_ExtensionMethodWithOptionalNullableDoubleParameter
{
throw new NotImplementedException();
}

[CakeMethodAlias]
public static void NonGeneric_ExtensionMethodWithReservedKeywordParameter(this ICakeContext context, int @new)
{
throw new NotImplementedException();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
public void NonGeneric_ExtensionMethodWithReservedKeywordParameter(System.Int32 @new)
{
Cake.Core.Tests.Data.MethodAliasGeneratorData.NonGeneric_ExtensionMethodWithReservedKeywordParameter(Context, @new);
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ public void Should_Throw_If_Method_Is_Null()
[InlineData("NonGeneric_ExtensionMethodWithOptionalNullableDecimalParameter")]
[InlineData("NonGeneric_ExtensionMethodWithOptionalNullableLongParameter")]
[InlineData("NonGeneric_ExtensionMethodWithOptionalNullableDoubleParameter")]
[InlineData("NonGeneric_ExtensionMethodWithReservedKeywordParameter")]
public void Should_Return_Correct_Generated_Code_For_Non_Generic_Methods(string name)
{
// Given
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,22 @@ public static void OptionalNullableEnumWithNonNullDefault(TestEnum? arg = TestEn
public static void OptionalNullableBoolWithNonNullDefault(bool? arg = true)
{
}

public static void RequiredIntKeyword(int @new)
{
}

public static void RequiredNullableIntKeyword(int? @new)
{
}

public static void OptionalIntKeywordWithNonNullDefault(int @new = 1)
{
}

public static void OptionalNullableIntKeywordWithNullDefault(int? @new = null)
{
}
}

[Theory]
Expand Down Expand Up @@ -424,6 +440,10 @@ public static void OptionalNullableBoolWithNonNullDefault(bool? arg = true)
[InlineData("OptionalNullableFloatWithNonNullDefault", "System.Nullable<System.Single> arg = (System.Single)1")]
[InlineData("OptionalNullableEnumWithNonNullDefault", "System.Nullable<Cake.Core.Tests.Unit.Scripting.CodeGen.ParameterEmitterTests+TestEnum> arg = (Cake.Core.Tests.Unit.Scripting.CodeGen.ParameterEmitterTests+TestEnum)1")]
[InlineData("OptionalNullableBoolWithNonNullDefault", "System.Nullable<System.Boolean> arg = (System.Boolean)true")]
[InlineData("RequiredIntKeyword", "System.Int32 @new")]
[InlineData("RequiredNullableIntKeyword", "System.Nullable<System.Int32> @new")]
[InlineData("OptionalIntKeywordWithNonNullDefault", "System.Int32 @new = (System.Int32)1")]
[InlineData("OptionalNullableIntKeywordWithNullDefault", "System.Nullable<System.Int32> @new = null")]
public void Should_Return_Correct_Generated_Code_For_Method_Parameters(string methodName, string expected)
{
// Given
Expand Down
137 changes: 137 additions & 0 deletions src/Cake.Core.Tests/Unit/Scripting/CodeGen/ParameterFormatterTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
// 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.Reflection;
using Cake.Core.Scripting.CodeGen;
using Xunit;

namespace Cake.Core.Tests.Unit.Scripting.CodeGen
{
public sealed class ParameterFormatterTests
{
private readonly ParameterFormatter _parameterFormatter = new ParameterFormatter();

[Fact]
public void Should_Throw_If_Parameter_Info_Is_Null()
{
// When
var result = Record.Exception(() => _parameterFormatter.FormatName((ParameterInfo)null));

// Then
Assert.IsArgumentNullException(result, "parameterInfo");
}

[Theory]
[InlineData(null)]
[InlineData("")]
[InlineData("\t")]
[InlineData(" ")]
public void Should_Throw_If_Parameter_Name_Is_Null_Or_Whitespace(string arg)
{
// When
var result = Record.Exception(() => _parameterFormatter.FormatName(arg));

// Then
Assert.IsArgumentException(result, "parameterName", "Parameter name cannot be null or whitespace");
}

[Theory]
[InlineData("abstract", "@abstract")]
[InlineData("as", "@as")]
[InlineData("base", "@base")]
[InlineData("bool", "@bool")]
[InlineData("break", "@break")]
[InlineData("byte", "@byte")]
[InlineData("case", "@case")]
[InlineData("catch", "@catch")]
[InlineData("char", "@char")]
[InlineData("checked", "@checked")]
[InlineData("class", "@class")]
[InlineData("const", "@const")]
[InlineData("continue", "@continue")]
[InlineData("decimal", "@decimal")]
[InlineData("default", "@default")]
[InlineData("delegate", "@delegate")]
[InlineData("do", "@do")]
[InlineData("double", "@double")]
[InlineData("else", "@else")]
[InlineData("enum", "@enum")]
[InlineData("event", "@event")]
[InlineData("explicit", "@explicit")]
[InlineData("extern", "@extern")]
[InlineData("false", "@false")]
[InlineData("finally", "@finally")]
[InlineData("fixed", "@fixed")]
[InlineData("float", "@float")]
[InlineData("for", "@for")]
[InlineData("foreach", "@foreach")]
[InlineData("goto", "@goto")]
[InlineData("if", "@if")]
[InlineData("implicit", "@implicit")]
[InlineData("in", "@in")]
[InlineData("int", "@int")]
[InlineData("interface", "@interface")]
[InlineData("internal", "@internal")]
[InlineData("is", "@is")]
[InlineData("lock", "@lock")]
[InlineData("long", "@long")]
[InlineData("namespace", "@namespace")]
[InlineData("new", "@new")]
[InlineData("null", "@null")]
[InlineData("object", "@object")]
[InlineData("operator", "@operator")]
[InlineData("out", "@out")]
[InlineData("override", "@override")]
[InlineData("params", "@params")]
[InlineData("private", "@private")]
[InlineData("protected", "@protected")]
[InlineData("public", "@public")]
[InlineData("readonly", "@readonly")]
[InlineData("ref", "@ref")]
[InlineData("return", "@return")]
[InlineData("sbyte", "@sbyte")]
[InlineData("sealed", "@sealed")]
[InlineData("short", "@short")]
[InlineData("sizeof", "@sizeof")]
[InlineData("stackalloc", "@stackalloc")]
[InlineData("static", "@static")]
[InlineData("string", "@string")]
[InlineData("struct", "@struct")]
[InlineData("switch", "@switch")]
[InlineData("this", "@this")]
[InlineData("throw", "@throw")]
[InlineData("true", "@true")]
[InlineData("try", "@try")]
[InlineData("typeof", "@typeof")]
[InlineData("uint", "@uint")]
[InlineData("ulong", "@ulong")]
[InlineData("unchecked", "@unchecked")]
[InlineData("unsafe", "@unsafe")]
[InlineData("ushort", "@ushort")]
[InlineData("using", "@using")]
[InlineData("virtual", "@virtual")]
[InlineData("void", "@void")]
[InlineData("volatile", "@volatile")]
[InlineData("while", "@while")]
public void Should_Format_Reserved_Keywords_Correctly(string parameterName, string expectedParameterName)
{
// When
var result = _parameterFormatter.FormatName(parameterName);

// Then
Assert.Equal(expectedParameterName, result);
}

[Theory]
[InlineData("testParameter", "testParameter")]
public void Should_Format_Variable_Names_Correctly(string parameterName, string expectedParameterName)
{
// When
var result = _parameterFormatter.FormatName(parameterName);

// Then
Assert.Equal(expectedParameterName, result);
}
}
}
5 changes: 4 additions & 1 deletion src/Cake.Core/Scripting/CodeGen/ParameterEmitter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ namespace Cake.Core.Scripting.CodeGen
/// </summary>
internal sealed class ParameterEmitter
{
private static readonly ParameterFormatter _parameterFormatter = new ParameterFormatter();

internal static string Emit(ParameterInfo parameter, bool includeType)
{
return string.Join(string.Empty, BuildParameterTokens(parameter, includeType));
Expand All @@ -39,7 +41,8 @@ private static IEnumerable<string> BuildParameterTokens(ParameterInfo parameter,
yield return parameter.ParameterType.GetFullName();
yield return " ";
}
yield return parameter.Name;

yield return _parameterFormatter.FormatName(parameter);

// GH-1166; add support for specifying default parameter values
if (includeType && parameter.IsOptional)
Expand Down
60 changes: 60 additions & 0 deletions src/Cake.Core/Scripting/CodeGen/ParameterFormatter.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
// 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.Generic;
using System.Reflection;

namespace Cake.Core.Scripting.CodeGen
{
/// <summary>
/// Provides support for cleaning parameter names consisting of reserved keywords
/// </summary>
internal sealed class ParameterFormatter
{
private readonly HashSet<string> _keywords;

internal ParameterFormatter()
{
// https://msdn.microsoft.com/en-us/library/x53a06bb.aspx
// reserved keywords are a no-go, contextual keywords were added in later versions
// and were designed to allow for legacy code that might utilize them as variables/parameters
// to work. i.e., where, yield, etc.
_keywords = new HashSet<string>(StringComparer.Ordinal)
{
"abstract", "as", "base", "bool", "break", "byte", "case",
"catch", "char", "checked", "class", "const", "continue",
"decimal", "default", "delegate", "do", "double", "else", "enum",
"event", "explicit", "extern", "false", "finally", "fixed", "float",
"for", "foreach", "goto", "if", "implicit", "in", "int", "interface",
"internal", "is", "lock", "long", "namespace", "new", "null", "object",
"operator", "out", "override", "params", "private", "protected",
"public", "readonly", "ref", "return", "sbyte", "sealed", "short",
"sizeof", "stackalloc", "static", "string", "struct", "switch",
"this", "throw", "true", "try", "typeof", "uint", "ulong", "unchecked",
"unsafe", "ushort", "using", "virtual", "void", "volatile", "while"
};
}

internal string FormatName(ParameterInfo parameterInfo)
{
if (parameterInfo == null)
{
throw new ArgumentNullException(nameof(parameterInfo));
}

return FormatName(parameterInfo.Name);
}

internal string FormatName(string parameterName)
{
if (string.IsNullOrWhiteSpace(parameterName))
{
throw new ArgumentException("Parameter name cannot be null or whitespace", nameof(parameterName));
}

return _keywords.Contains(parameterName) ? $"@{parameterName}" : parameterName;
}
}
}

0 comments on commit 70ba5e1

Please sign in to comment.