From c3a31c9c81c0fb5db71e194d24c27df16336bb91 Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Wed, 8 Aug 2012 18:18:49 +0200 Subject: [PATCH] Fix #92: The resolver does not check type constraints on calls to generic methods --- .../Resolver/OverloadResolution.cs | 107 +++++++++++++----- .../Resolver/OverloadResolutionErrors.cs | 15 ++- .../CSharp/Parser/ParseSelfTests.cs | 2 +- .../CSharp/Resolver/InvocationTests.cs | 28 ++++- 4 files changed, 123 insertions(+), 29 deletions(-) diff --git a/ICSharpCode.NRefactory.CSharp/Resolver/OverloadResolution.cs b/ICSharpCode.NRefactory.CSharp/Resolver/OverloadResolution.cs index ba407c76fa..e94ffa42a2 100644 --- a/ICSharpCode.NRefactory.CSharp/Resolver/OverloadResolution.cs +++ b/ICSharpCode.NRefactory.CSharp/Resolver/OverloadResolution.cs @@ -118,7 +118,8 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver public void AddError(OverloadResolutionErrors newError) { this.Errors |= newError; - this.ErrorCount++; + if (!IsApplicable(newError)) + this.ErrorCount++; } } @@ -130,6 +131,8 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver Candidate bestCandidate; Candidate bestCandidateAmbiguousWith; IType[] explicitlyGivenTypeArguments; + bool bestCandidateWasValidated; + OverloadResolutionErrors bestCandidateValidationResult; #region Constructor public OverloadResolution(ICompilation compilation, ResolveResult[] arguments, string[] argumentNames = null, IType[] typeArguments = null, CSharpConversions conversions = null) @@ -155,6 +158,7 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver } #endregion + #region Input Properties /// /// Gets/Sets whether the methods are extension methods that are being called using extension method syntax. /// @@ -183,21 +187,36 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver public IList Arguments { get { return arguments; } } + #endregion #region AddCandidate + /// + /// Adds a candidate to overload resolution. + /// + /// The candidate member to add. + /// The errors that prevent the member from being applicable, if any. + /// Note: this method does not return errors that do not affect applicability. public OverloadResolutionErrors AddCandidate(IParameterizedMember member) { return AddCandidate(member, OverloadResolutionErrors.None); } + /// + /// Adds a candidate to overload resolution. + /// + /// The candidate member to add. + /// Additional errors that apply to the candidate. + /// This is used to represent errors during member lookup (e.g. OverloadResolutionErrors.Inaccessible) + /// in overload resolution. + /// The errors that prevent the member from being applicable, if any. + /// Note: this method does not return errors that do not affect applicability. public OverloadResolutionErrors AddCandidate(IParameterizedMember member, OverloadResolutionErrors additionalErrors) { if (member == null) throw new ArgumentNullException("member"); Candidate c = new Candidate(member, false); - if (additionalErrors != OverloadResolutionErrors.None) - c.AddError(additionalErrors); + c.AddError(additionalErrors); if (CalculateCandidate(c)) { //candidates.Add(c); } @@ -206,8 +225,7 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver && member.Parameters[member.Parameters.Count - 1].IsParams) { Candidate expandedCandidate = new Candidate(member, true); - if (additionalErrors != OverloadResolutionErrors.None) - expandedCandidate.AddError(additionalErrors); + expandedCandidate.AddError(additionalErrors); // consider expanded form only if it isn't obviously wrong if (CalculateCandidate(expandedCandidate)) { //candidates.Add(expandedCandidate); @@ -219,11 +237,6 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver return c.Errors; } - public static bool IsApplicable(OverloadResolutionErrors errors) - { - return (errors & ~OverloadResolutionErrors.AmbiguousMatch) == OverloadResolutionErrors.None; - } - /// /// Calculates applicability etc. for the candidate. /// @@ -236,6 +249,7 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver RunTypeInference(candidate); CheckApplicability(candidate); ConsiderIfNewCandidateIsBest(candidate); + ValidateMethodConstraints(candidate); return true; } @@ -432,20 +446,36 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver #endregion #region Validate Constraints + OverloadResolutionErrors ValidateMethodConstraints(Candidate candidate) + { + // If type inference already failed, we won't check the constraints: + if ((candidate.Errors & OverloadResolutionErrors.TypeInferenceFailed) != 0) + return OverloadResolutionErrors.None; + + IMethod method = candidate.Member as IMethod; + if (method == null || method.TypeParameters.Count == 0) + return OverloadResolutionErrors.None; // the method isn't generic + var substitution = GetSubstitution(candidate); + for (int i = 0; i < method.TypeParameters.Count; i++) { + if (!ValidateConstraints(method.TypeParameters[i], substitution.MethodTypeArguments[i], substitution)) + return OverloadResolutionErrors.MethodConstraintsNotSatisfied; + } + return OverloadResolutionErrors.None; + } + /// /// Validates whether the given type argument satisfies the constraints for the given type parameter. /// /// The type parameter. /// The type argument. /// The substitution that defines how type parameters are replaced with type arguments. - /// The substitution is used to check constraints that depend on other type parameters (or recursively on the same type parameter). + /// The substitution is used to check constraints that depend on other type parameters (or recursively on the same type parameter). + /// May be null if no substitution should be used. /// True if the constraints are satisfied; false otherwise. - public static bool ValidateConstraints(ITypeParameter typeParameter, IType typeArgument, TypeVisitor substitution) + public static bool ValidateConstraints(ITypeParameter typeParameter, IType typeArgument, TypeVisitor substitution = null) { if (typeParameter == null) throw new ArgumentNullException("typeParameter"); - if (typeParameter.Owner == null) - throw new ArgumentNullException("typeParameter.Owner"); if (typeArgument == null) throw new ArgumentNullException("typeArgument"); return ValidateConstraints(typeParameter, typeArgument, substitution, CSharpConversions.Get(typeParameter.Owner.Compilation)); @@ -490,6 +520,16 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver #endregion #region CheckApplicability + /// + /// Returns whether a candidate with the given errors is still considered to be applicable. + /// + public static bool IsApplicable(OverloadResolutionErrors errors) + { + const OverloadResolutionErrors errorsThatDoNotMatterForApplicability = + OverloadResolutionErrors.AmbiguousMatch | OverloadResolutionErrors.MethodConstraintsNotSatisfied; + return (errors & ~errorsThatDoNotMatterForApplicability) == OverloadResolutionErrors.None; + } + void CheckApplicability(Candidate candidate) { // C# 4.0 spec: ยง7.5.3.1 Applicable function member @@ -697,6 +737,7 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver { if (bestCandidate == null) { bestCandidate = candidate; + bestCandidateWasValidated = false; } else { switch (BetterFunctionMember(candidate, bestCandidate)) { case 0: @@ -707,6 +748,7 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver break; case 1: bestCandidate = candidate; + bestCandidateWasValidated = false; bestCandidateAmbiguousWith = null; break; // case 2: best candidate stays best @@ -715,15 +757,24 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver } #endregion + #region Output Properties public IParameterizedMember BestCandidate { get { return bestCandidate != null ? bestCandidate.Member : null; } } + /// + /// Returns the errors that apply to the best candidate. + /// This includes additional errors that do not affect applicability (e.g. AmbiguousMatch, MethodConstraintsNotSatisfied) + /// public OverloadResolutionErrors BestCandidateErrors { get { if (bestCandidate == null) return OverloadResolutionErrors.None; - OverloadResolutionErrors err = bestCandidate.Errors; + if (!bestCandidateWasValidated) { + bestCandidateValidationResult = ValidateMethodConstraints(bestCandidate); + bestCandidateWasValidated = true; + } + OverloadResolutionErrors err = bestCandidate.Errors | bestCandidateValidationResult; if (bestCandidateAmbiguousWith != null) err |= OverloadResolutionErrors.AmbiguousMatch; return err; @@ -731,7 +782,7 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver } public bool FoundApplicableCandidate { - get { return bestCandidate != null && bestCandidate.Errors == OverloadResolutionErrors.None; } + get { return bestCandidate != null && IsApplicable(bestCandidate.Errors); } } public IParameterizedMember BestCandidateAmbiguousWith { @@ -848,21 +899,24 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver return null; IMethod method = bestCandidate.Member as IMethod; if (method != null && method.TypeParameters.Count > 0) { - SpecializedMethod sm = method as SpecializedMethod; - if (sm != null) { - // Do not compose the substitutions, but merge them. - // This is required for InvocationTests.SubstituteClassAndMethodTypeParametersAtOnce - return new SpecializedMethod( - (IMethod)method.MemberDefinition, - new TypeParameterSubstitution(sm.Substitution.ClassTypeArguments, bestCandidate.InferredTypes)); - } else { - return new SpecializedMethod(method, new TypeParameterSubstitution(null, bestCandidate.InferredTypes)); - } + return new SpecializedMethod((IMethod)method.MemberDefinition, GetSubstitution(bestCandidate)); } else { return bestCandidate.Member; } } + TypeParameterSubstitution GetSubstitution(Candidate candidate) + { + SpecializedMethod sm = candidate.Member as SpecializedMethod; + if (sm != null) { + // Do not compose the substitutions, but merge them. + // This is required for InvocationTests.SubstituteClassAndMethodTypeParametersAtOnce + return new TypeParameterSubstitution(sm.Substitution.ClassTypeArguments, candidate.InferredTypes); + } else { + return new TypeParameterSubstitution(null, candidate.InferredTypes); + } + } + /// /// Creates a ResolveResult representing the result of overload resolution. /// @@ -890,5 +944,6 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver argumentToParameterMap: this.GetArgumentToParameterMap(), initializerStatements: initializerStatements); } + #endregion } } diff --git a/ICSharpCode.NRefactory.CSharp/Resolver/OverloadResolutionErrors.cs b/ICSharpCode.NRefactory.CSharp/Resolver/OverloadResolutionErrors.cs index 1b1ea2c91c..399f2ff091 100644 --- a/ICSharpCode.NRefactory.CSharp/Resolver/OverloadResolutionErrors.cs +++ b/ICSharpCode.NRefactory.CSharp/Resolver/OverloadResolutionErrors.cs @@ -65,10 +65,23 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver /// There is no unique best overload. /// This error does not apply to any single candidate, but only to the overall result of overload resolution. /// + /// + /// This error does not prevent a candidate from being applicable. + /// AmbiguousMatch = 0x0200, /// /// The member is not accessible. /// - Inaccessible = 0x0400 + /// + /// This error is generated by member lookup; not by overload resolution. + /// + Inaccessible = 0x0400, + /// + /// A generic method + /// + /// + /// This error does not prevent a candidate from being applicable. + /// + MethodConstraintsNotSatisfied = 0x0800 } } diff --git a/ICSharpCode.NRefactory.Tests/CSharp/Parser/ParseSelfTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/Parser/ParseSelfTests.cs index 2bc9f88760..54abb3fb5d 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/Parser/ParseSelfTests.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/Parser/ParseSelfTests.cs @@ -37,7 +37,7 @@ namespace ICSharpCode.NRefactory.CSharp.Parser [TestFixtureSetUp] public void SetUp() { - string path = Path.GetFullPath (Path.Combine ("..", "..", "..")); + string path = Path.GetFullPath (Path.Combine ("..", "..")); if (!File.Exists(Path.Combine(path, "NRefactory.sln"))) throw new InvalidOperationException("Test cannot find the NRefactory source code in " + path); fileNames = Directory.GetFiles(path, "*.cs", SearchOption.AllDirectories); diff --git a/ICSharpCode.NRefactory.Tests/CSharp/Resolver/InvocationTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/Resolver/InvocationTests.cs index 6f1946dd3a..5814490df3 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/Resolver/InvocationTests.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/Resolver/InvocationTests.cs @@ -598,7 +598,7 @@ class Test { [Test] public void NamedArgumentInMissingMethod() { - string program = @" + string program = @" class Test { public void Test() { Missing($x: 0$); @@ -609,5 +609,31 @@ class Test { Assert.AreEqual("x", narr.ParameterName); Assert.IsNull(narr.Parameter); } + + [Test] + public void GenericMethodInvocationWithConstraintMismatch() + { + string program = @" +interface IA +{ +} +class Test +{ + void F() + { + string o = null; + $M(o)$; + } + + void M(T arg) where T : IA + { + } + void M(object arg) { + } +}"; + var rr = Resolve(program); + Assert.AreEqual(OverloadResolutionErrors.MethodConstraintsNotSatisfied, rr.OverloadResolutionErrors); + Assert.IsTrue(rr.IsError); + } } }