From e39bc70714e88d716c1242c3c1d0aa2c420546ff Mon Sep 17 00:00:00 2001 From: Andreas Weizel Date: Sun, 5 Oct 2014 19:26:31 +0200 Subject: [PATCH 1/3] Fix: Interface events indirectly implemented by a base class were not recognized and marked as "missing" by MissingInterfaceMemberImplementationIssue. --- .../CodeActions/ImplementInterfaceAction.cs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/Libraries/NRefactory/ICSharpCode.NRefactory.CSharp.Refactoring/CodeActions/ImplementInterfaceAction.cs b/src/Libraries/NRefactory/ICSharpCode.NRefactory.CSharp.Refactoring/CodeActions/ImplementInterfaceAction.cs index 625b49bf30..76261d9a18 100644 --- a/src/Libraries/NRefactory/ICSharpCode.NRefactory.CSharp.Refactoring/CodeActions/ImplementInterfaceAction.cs +++ b/src/Libraries/NRefactory/ICSharpCode.NRefactory.CSharp.Refactoring/CodeActions/ImplementInterfaceAction.cs @@ -131,8 +131,21 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring continue; bool needsExplicitly = explicitly; - alreadyImplemented = implementingType.GetMembers().Any(m => m.ImplementedInterfaceMembers.Any(im => IsImplementation (im, ev))); + alreadyImplemented = false; + + foreach (var cmet in implementingType.GetMembers ()) { + alreadyImplemented |= cmet.ImplementedInterfaceMembers.Any(m => IsImplementation (m, ev)); + + if (CompareMembers(ev, cmet)) { + if (!needsExplicitly && !cmet.ReturnType.Equals(ev.ReturnType)) + needsExplicitly = true; + else + alreadyImplemented |= !needsExplicitly /*|| cmet.InterfaceImplementations.Any (impl => impl.InterfaceType.Equals (interfaceType))*/; + } + } + if (toImplement.Where(t => t.Item1 is IEvent).Any(t => CompareMembers(ev, (IEvent)t.Item1))) + needsExplicitly = true; if (!alreadyImplemented) { toImplement.Add(new Tuple(ev, needsExplicitly)); } else { From 3e5018a49d39a4562e5ce7e0a71ab5b9b271b2d3 Mon Sep 17 00:00:00 2001 From: Andreas Weizel Date: Sat, 11 Oct 2014 00:36:36 +0200 Subject: [PATCH 2/3] Changed ImplementInterfaceAction to use ITypeDefinition.GetInterfaceImplementation(). --- .../CodeActions/ImplementInterfaceAction.cs | 90 ++++++++++--------- 1 file changed, 46 insertions(+), 44 deletions(-) diff --git a/src/Libraries/NRefactory/ICSharpCode.NRefactory.CSharp.Refactoring/CodeActions/ImplementInterfaceAction.cs b/src/Libraries/NRefactory/ICSharpCode.NRefactory.CSharp.Refactoring/CodeActions/ImplementInterfaceAction.cs index 76261d9a18..7c996ae2bb 100644 --- a/src/Libraries/NRefactory/ICSharpCode.NRefactory.CSharp.Refactoring/CodeActions/ImplementInterfaceAction.cs +++ b/src/Libraries/NRefactory/ICSharpCode.NRefactory.CSharp.Refactoring/CodeActions/ImplementInterfaceAction.cs @@ -117,7 +117,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring { return m.UnresolvedMember == method.UnresolvedMember; } - + public static List> CollectMembersToImplement(ITypeDefinition implementingType, IType interfaceType, bool explicitly, out bool interfaceMissing) { //var def = interfaceType.GetDefinition(); @@ -131,21 +131,9 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring continue; bool needsExplicitly = explicitly; - alreadyImplemented = false; - - foreach (var cmet in implementingType.GetMembers ()) { - alreadyImplemented |= cmet.ImplementedInterfaceMembers.Any(m => IsImplementation (m, ev)); - - if (CompareMembers(ev, cmet)) { - if (!needsExplicitly && !cmet.ReturnType.Equals(ev.ReturnType)) - needsExplicitly = true; - else - alreadyImplemented |= !needsExplicitly /*|| cmet.InterfaceImplementations.Any (impl => impl.InterfaceType.Equals (interfaceType))*/; - } - } - - if (toImplement.Where(t => t.Item1 is IEvent).Any(t => CompareMembers(ev, (IEvent)t.Item1))) - needsExplicitly = true; + var implementingMember = implementingType.GetInterfaceImplementation(ev); + alreadyImplemented = (implementingMember != null) && + (GetAccessibilityLevel(implementingMember) <= GetAccessibilityLevel(ev)); if (!alreadyImplemented) { toImplement.Add(new Tuple(ev, needsExplicitly)); } else { @@ -160,17 +148,19 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring continue; bool needsExplicitly = explicitly; alreadyImplemented = false; - - foreach (var cmet in implementingType.GetMethods ()) { - alreadyImplemented |= cmet.ImplementedInterfaceMembers.Any(m => IsImplementation (m, method)); - - if (CompareMembers(method, cmet)) { - if (!needsExplicitly && !cmet.ReturnType.Equals(method.ReturnType)) - needsExplicitly = true; - else - alreadyImplemented |= !needsExplicitly /*|| cmet.InterfaceImplementations.Any (impl => impl.InterfaceType.Equals (interfaceType))*/; + + var implementingMethod = implementingType.GetInterfaceImplementation(method); + alreadyImplemented = (implementingMethod != null) && + (GetAccessibilityLevel(implementingMethod) <= GetAccessibilityLevel(method)); + if (alreadyImplemented) { + if (!needsExplicitly && !implementingMethod.ReturnType.Equals(method.ReturnType)) { + needsExplicitly = true; + alreadyImplemented = false; + } else { + alreadyImplemented = !needsExplicitly; } } + if (toImplement.Where(t => t.Item1 is IMethod).Any(t => CompareMembers(method, (IMethod)t.Item1))) needsExplicitly = true; if (!alreadyImplemented) { @@ -187,27 +177,19 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring continue; bool needsExplicitly = explicitly; - alreadyImplemented = implementingType.GetMembers().Any(m => m.ImplementedInterfaceMembers.Any(im => IsImplementation (im, prop))); - - foreach (var t in implementingType.GetAllBaseTypeDefinitions ()) { - if (t.Kind == TypeKind.Interface) { - foreach (var cprop in t.Properties) { - if (cprop.Name == prop.Name && cprop.IsShadowing) { - if (!needsExplicitly && !cprop.ReturnType.Equals(prop.ReturnType)) - needsExplicitly = true; - } - } - continue; - } - foreach (var cprop in t.Properties) { - if (cprop.Name == prop.Name) { - if (!needsExplicitly && !cprop.ReturnType.Equals(prop.ReturnType)) - needsExplicitly = true; - else - alreadyImplemented |= !needsExplicitly/* || cprop.InterfaceImplementations.Any (impl => impl.InterfaceType.Resolve (ctx).Equals (interfaceType))*/; - } + + var implementingProp = implementingType.GetInterfaceImplementation(prop); + alreadyImplemented = (implementingProp != null) && + (GetAccessibilityLevel(implementingProp) <= GetAccessibilityLevel(prop)); + if (alreadyImplemented) { + if (!needsExplicitly && !implementingProp.ReturnType.Equals(prop.ReturnType)) { + needsExplicitly = true; + alreadyImplemented = false; + } else { + alreadyImplemented = !needsExplicitly; } } + if (!alreadyImplemented) { toImplement.Add(new Tuple(prop, needsExplicitly)); } else { @@ -217,6 +199,26 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring return toImplement; } + static int GetAccessibilityLevel(IHasAccessibility member) + { + if (member.Accessibility == Accessibility.None) + return 0; + if (member.IsPublic) + return 1; + if (member.IsInternal) + return 2; + if (member.IsProtectedOrInternal) + return 3; + if (member.IsProtectedAndInternal) + return 4; + if (member.IsProtected) + return 5; + if (member.IsPrivate) + return 6; + + return 7; + } + internal static bool CompareMembers(IMember interfaceMethod, IMember typeMethod) { if (typeMethod.IsExplicitInterfaceImplementation) From 7b9b255637cace9877ae37cf25dc28e26a1bb94c Mon Sep 17 00:00:00 2001 From: Andreas Weizel Date: Sat, 25 Oct 2014 01:07:27 +0200 Subject: [PATCH 3/3] Removed comparison of "visibility levels" for now. --- .../CodeActions/ImplementInterfaceAction.cs | 29 ++----------------- 1 file changed, 3 insertions(+), 26 deletions(-) diff --git a/src/Libraries/NRefactory/ICSharpCode.NRefactory.CSharp.Refactoring/CodeActions/ImplementInterfaceAction.cs b/src/Libraries/NRefactory/ICSharpCode.NRefactory.CSharp.Refactoring/CodeActions/ImplementInterfaceAction.cs index 7c996ae2bb..53cd356bc6 100644 --- a/src/Libraries/NRefactory/ICSharpCode.NRefactory.CSharp.Refactoring/CodeActions/ImplementInterfaceAction.cs +++ b/src/Libraries/NRefactory/ICSharpCode.NRefactory.CSharp.Refactoring/CodeActions/ImplementInterfaceAction.cs @@ -132,8 +132,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring bool needsExplicitly = explicitly; var implementingMember = implementingType.GetInterfaceImplementation(ev); - alreadyImplemented = (implementingMember != null) && - (GetAccessibilityLevel(implementingMember) <= GetAccessibilityLevel(ev)); + alreadyImplemented = (implementingMember != null); if (!alreadyImplemented) { toImplement.Add(new Tuple(ev, needsExplicitly)); } else { @@ -150,8 +149,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring alreadyImplemented = false; var implementingMethod = implementingType.GetInterfaceImplementation(method); - alreadyImplemented = (implementingMethod != null) && - (GetAccessibilityLevel(implementingMethod) <= GetAccessibilityLevel(method)); + alreadyImplemented = (implementingMethod != null); if (alreadyImplemented) { if (!needsExplicitly && !implementingMethod.ReturnType.Equals(method.ReturnType)) { needsExplicitly = true; @@ -179,8 +177,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring bool needsExplicitly = explicitly; var implementingProp = implementingType.GetInterfaceImplementation(prop); - alreadyImplemented = (implementingProp != null) && - (GetAccessibilityLevel(implementingProp) <= GetAccessibilityLevel(prop)); + alreadyImplemented = (implementingProp != null); if (alreadyImplemented) { if (!needsExplicitly && !implementingProp.ReturnType.Equals(prop.ReturnType)) { needsExplicitly = true; @@ -199,26 +196,6 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring return toImplement; } - static int GetAccessibilityLevel(IHasAccessibility member) - { - if (member.Accessibility == Accessibility.None) - return 0; - if (member.IsPublic) - return 1; - if (member.IsInternal) - return 2; - if (member.IsProtectedOrInternal) - return 3; - if (member.IsProtectedAndInternal) - return 4; - if (member.IsProtected) - return 5; - if (member.IsPrivate) - return 6; - - return 7; - } - internal static bool CompareMembers(IMember interfaceMethod, IMember typeMethod) { if (typeMethod.IsExplicitInterfaceImplementation)