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

Revert "Permit throwaway parameters (#10209)" #10436

Merged
merged 1 commit into from
Jul 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
33 changes: 0 additions & 33 deletions src/Build.UnitTests/Evaluation/Expander_Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1294,39 +1294,6 @@ public void StaticMethodErrorMessageHaveMethodName1()

Assert.True(false);
}

[Fact]
public void StaticMethodWithThrowawayParameterSupported()
{
MockLogger logger = Helpers.BuildProjectWithNewOMExpectSuccess(@"
<Project>
<PropertyGroup>
<MyProperty>Value is $([System.Int32]::TryParse(""3"", _))</MyProperty>
</PropertyGroup>
<Target Name='Build'>
<Message Text='$(MyProperty)' />
</Target>
</Project>");

logger.FullLog.ShouldContain("Value is True");
}

[Fact]
public void StaticMethodWithThrowawayParameterSupported2()
{
MockLogger logger = Helpers.BuildProjectWithNewOMExpectSuccess(@"
<Project>
<PropertyGroup>
<MyProperty>Value is $([System.Int32]::TryParse(""notANumber"", _))</MyProperty>
</PropertyGroup>
<Target Name='Build'>
<Message Text='$(MyProperty)' />
</Target>
</Project>");

logger.FullLog.ShouldContain("Value is False");
}

/// <summary>
/// Creates a set of complicated item metadata and properties, and items to exercise
/// the Expander class. The data here contains escaped characters, metadata that
Expand Down
55 changes: 2 additions & 53 deletions src/Build/Evaluation/Expander.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3593,17 +3593,8 @@ internal object Execute(object objectInstance, IPropertyProvider<T> properties,
// otherwise there is the potential of running a function twice!
try
{
// If there are any out parameters, try to figure out their type and create defaults for them as appropriate before calling the method.
if (args.Any(a => "_".Equals(a)))
{
IEnumerable<MethodInfo> methods = _receiverType.GetMethods(_bindingFlags).Where(m => m.Name.Equals(_methodMethodName) && m.GetParameters().Length == args.Length);
functionResult = GetMethodResult(objectInstance, methods, args, 0);
}
else
{
// If there are no out parameters, use InvokeMember using the standard binder - this will match and coerce as needed
functionResult = _receiverType.InvokeMember(_methodMethodName, _bindingFlags, Type.DefaultBinder, objectInstance, args, CultureInfo.InvariantCulture);
}
// First use InvokeMember using the standard binder - this will match and coerce as needed
functionResult = _receiverType.InvokeMember(_methodMethodName, _bindingFlags, Type.DefaultBinder, objectInstance, args, CultureInfo.InvariantCulture);
}
// If we're invoking a method, then there are deeper attempts that can be made to invoke the method.
// If not, we were asked to get a property or field but found that we cannot locate it. No further argument coercion is possible, so throw.
Expand Down Expand Up @@ -3678,48 +3669,6 @@ internal object Execute(object objectInstance, IPropertyProvider<T> properties,
}
}

private object GetMethodResult(object objectInstance, IEnumerable<MethodInfo> methods, object[] args, int index)
{
for (int i = index; i < args.Length; i++)
{
if (args[i].Equals("_"))
{
object toReturn = null;
foreach (MethodInfo method in methods)
{
Type t = method.GetParameters()[i].ParameterType;
args[i] = t.IsValueType ? Activator.CreateInstance(t) : null;
object currentReturnValue = GetMethodResult(objectInstance, methods, args, i + 1);
if (currentReturnValue is not null)
{
if (toReturn is null)
{
toReturn = currentReturnValue;
}
else if (!toReturn.Equals(currentReturnValue))
{
// There were multiple methods that seemed viable and gave different results. We can't differentiate between them so throw.
ErrorUtilities.ThrowArgument("CouldNotDifferentiateBetweenCompatibleMethods", _methodMethodName, args.Length);
return null;
}
}
}

return toReturn;
}
}

try
{
return _receiverType.InvokeMember(_methodMethodName, _bindingFlags, Type.DefaultBinder, objectInstance, args, CultureInfo.InvariantCulture);
}
catch (Exception)
{
// This isn't a viable option, but perhaps another set of parameters will work.
return null;
}
}

/// <summary>
/// Shortcut to avoid calling into binding if we recognize some most common functions.
/// Binding is expensive and throws first-chance MissingMethodExceptions, which is
Expand Down
3 changes: 0 additions & 3 deletions src/Build/Resources/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -604,9 +604,6 @@
LOCALIZATION: "{0}" is the expression that was bad. "{1}" is a message from an FX exception that describes why the expression is bad.
</comment>
</data>
<data name="CouldNotDifferentiateBetweenCompatibleMethods">
<value>Found multiple overloads for method "{0}" with {1} parameter(s). That is currently not supported.</value>
</data>
<data name="InvalidFunctionPropertyExpression" xml:space="preserve">
<value>MSB4184: The expression "{0}" cannot be evaluated. {1}</value>
<comment>{StrBegin="MSB4184: "}
Expand Down
5 changes: 0 additions & 5 deletions src/Build/Resources/xlf/Strings.cs.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 0 additions & 5 deletions src/Build/Resources/xlf/Strings.de.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 0 additions & 5 deletions src/Build/Resources/xlf/Strings.es.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 0 additions & 5 deletions src/Build/Resources/xlf/Strings.fr.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 0 additions & 5 deletions src/Build/Resources/xlf/Strings.it.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 0 additions & 5 deletions src/Build/Resources/xlf/Strings.ja.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 0 additions & 5 deletions src/Build/Resources/xlf/Strings.ko.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 0 additions & 5 deletions src/Build/Resources/xlf/Strings.pl.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 0 additions & 5 deletions src/Build/Resources/xlf/Strings.pt-BR.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 0 additions & 5 deletions src/Build/Resources/xlf/Strings.ru.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 0 additions & 5 deletions src/Build/Resources/xlf/Strings.tr.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 0 additions & 5 deletions src/Build/Resources/xlf/Strings.zh-Hans.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 0 additions & 5 deletions src/Build/Resources/xlf/Strings.zh-Hant.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading