From 6d0f3fb02ec076603062b8e84573d0df94e69d71 Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Thu, 3 Jan 2013 22:31:55 +0100 Subject: [PATCH] Add "add using" context action. The unit tests are based on pull request #104 by Adam Connelly --- ICSharpCode.NRefactory.CSharp/Ast/AstType.cs | 15 + .../Ast/GeneralScope/Comment.cs | 9 + .../Ast/GeneralScope/UsingDeclaration.cs | 2 +- .../Formatter/AstFormattingVisitor.cs | 1 + .../Formatter/CSharpFormattingOptions.cs | 14 +- .../Formatter/FormattingOptionsFactory.cs | 4 +- .../ICSharpCode.NRefactory.CSharp.csproj | 2 + .../Refactoring/CodeActions/AddUsingAction.cs | 156 +++++++++ .../CodeActions/CreateFieldAction.cs | 4 +- .../CodeActions/SortUsingsAction.cs | 75 +--- .../Refactoring/CodeIssue.cs | 2 +- .../CodeIssues/ExceptionRethrowIssue.cs | 2 +- .../CodeIssues/GatherVisitorBase.cs | 17 +- ...ncorrectExceptionParameterOrderingIssue.cs | 6 +- ...erenceToStaticMemberViaDerivedTypeIssue.cs | 6 +- ...esultOfAsyncCallShouldNotBeIgnoredIssue.cs | 2 +- .../Refactoring/Script.cs | 22 +- .../Refactoring/UsingHelper.cs | 180 ++++++++++ .../TypeSystem/TypeSystemConvertVisitor.cs | 2 +- .../AddUsingActionAlphabeticalTests.cs | 229 +++++++++++++ .../AddUsingActionInsideNamespaceTests.cs | 203 +++++++++++ .../AddUsing/AddUsingActionTests.cs | 320 ++++++++++++++++++ .../AddUsing/AddUsingRunActionTests.cs | 248 ++++++++++++++ .../CodeActions/ContextActionTestBase.cs | 40 ++- .../CSharp/CodeActions/SortUsingsTests.cs | 17 + .../CodeActions/TestRefactoringContext.cs | 5 +- .../TestStatementIndentation.cs | 23 ++ .../ICSharpCode.NRefactory.Tests.csproj | 5 + 28 files changed, 1495 insertions(+), 116 deletions(-) create mode 100644 ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/AddUsingAction.cs create mode 100644 ICSharpCode.NRefactory.CSharp/Refactoring/UsingHelper.cs create mode 100644 ICSharpCode.NRefactory.Tests/CSharp/CodeActions/AddUsing/AddUsingActionAlphabeticalTests.cs create mode 100644 ICSharpCode.NRefactory.Tests/CSharp/CodeActions/AddUsing/AddUsingActionInsideNamespaceTests.cs create mode 100644 ICSharpCode.NRefactory.Tests/CSharp/CodeActions/AddUsing/AddUsingActionTests.cs create mode 100644 ICSharpCode.NRefactory.Tests/CSharp/CodeActions/AddUsing/AddUsingRunActionTests.cs diff --git a/ICSharpCode.NRefactory.CSharp/Ast/AstType.cs b/ICSharpCode.NRefactory.CSharp/Ast/AstType.cs index ef6a52a9a1..0e93bc186d 100644 --- a/ICSharpCode.NRefactory.CSharp/Ast/AstType.cs +++ b/ICSharpCode.NRefactory.CSharp/Ast/AstType.cs @@ -251,5 +251,20 @@ namespace ICSharpCode.NRefactory.CSharp { return new TypeReferenceExpression { Type = this }.Invoke(methodName, typeArguments, arguments); } + + /// + /// Creates a simple AstType from a dotted name. + /// Does not support generics, arrays, etc. - just simple dotted names, + /// e.g. namespace names. + /// + public static AstType Create(string dottedName) + { + string[] parts = dottedName.Split('.'); + AstType type = new SimpleType(parts[0]); + for (int i = 1; i < parts.Length; i++) { + type = new MemberType(type, parts[i]); + } + return type; + } } } diff --git a/ICSharpCode.NRefactory.CSharp/Ast/GeneralScope/Comment.cs b/ICSharpCode.NRefactory.CSharp/Ast/GeneralScope/Comment.cs index 83c00c9bf0..9d53f6e661 100644 --- a/ICSharpCode.NRefactory.CSharp/Ast/GeneralScope/Comment.cs +++ b/ICSharpCode.NRefactory.CSharp/Ast/GeneralScope/Comment.cs @@ -65,6 +65,15 @@ namespace ICSharpCode.NRefactory.CSharp set { ThrowIfFrozen(); commentType = value; } } + /// + /// Returns true if the is Documentation or MultiLineDocumentation. + /// + public bool IsDocumentation { + get { + return commentType == CommentType.Documentation || commentType == CommentType.MultiLineDocumentation; + } + } + bool startsLine; public bool StartsLine { diff --git a/ICSharpCode.NRefactory.CSharp/Ast/GeneralScope/UsingDeclaration.cs b/ICSharpCode.NRefactory.CSharp/Ast/GeneralScope/UsingDeclaration.cs index c5ae723512..eff582c338 100644 --- a/ICSharpCode.NRefactory.CSharp/Ast/GeneralScope/UsingDeclaration.cs +++ b/ICSharpCode.NRefactory.CSharp/Ast/GeneralScope/UsingDeclaration.cs @@ -67,7 +67,7 @@ namespace ICSharpCode.NRefactory.CSharp public UsingDeclaration (string nameSpace) { - AddChild (new SimpleType (nameSpace), ImportRole); + AddChild (AstType.Create (nameSpace), ImportRole); } public UsingDeclaration (AstType import) diff --git a/ICSharpCode.NRefactory.CSharp/Formatter/AstFormattingVisitor.cs b/ICSharpCode.NRefactory.CSharp/Formatter/AstFormattingVisitor.cs index 96356cbe4a..f8f12fa782 100644 --- a/ICSharpCode.NRefactory.CSharp/Formatter/AstFormattingVisitor.cs +++ b/ICSharpCode.NRefactory.CSharp/Formatter/AstFormattingVisitor.cs @@ -261,6 +261,7 @@ namespace ICSharpCode.NRefactory.CSharp public override void VisitUsingDeclaration(UsingDeclaration usingDeclaration) { if (usingDeclaration.PrevSibling != null && !(usingDeclaration.PrevSibling is UsingDeclaration || usingDeclaration.PrevSibling is UsingAliasDeclaration)) { + FixIndentationForceNewLine(usingDeclaration.StartLocation); EnsureBlankLinesBefore(usingDeclaration, policy.BlankLinesBeforeUsings); } else if (!(usingDeclaration.NextSibling is UsingDeclaration || usingDeclaration.NextSibling is UsingAliasDeclaration)) { FixIndentationForceNewLine(usingDeclaration.StartLocation); diff --git a/ICSharpCode.NRefactory.CSharp/Formatter/CSharpFormattingOptions.cs b/ICSharpCode.NRefactory.CSharp/Formatter/CSharpFormattingOptions.cs index 23152e5bde..acc7d165d8 100644 --- a/ICSharpCode.NRefactory.CSharp/Formatter/CSharpFormattingOptions.cs +++ b/ICSharpCode.NRefactory.CSharp/Formatter/CSharpFormattingOptions.cs @@ -1,4 +1,4 @@ -// +// // CSharpFormattingOptions.cs // // Author: @@ -68,6 +68,11 @@ namespace ICSharpCode.NRefactory.CSharp SameLine } + public enum UsingPlacement { + TopOfFile, + InsideNamespace + } + public class CSharpFormattingOptions { public string Name { @@ -859,6 +864,13 @@ namespace ICSharpCode.NRefactory.CSharp } #endregion + #region Using Declarations + public UsingPlacement UsingPlacement { + get; + set; + } + #endregion + internal CSharpFormattingOptions() { } diff --git a/ICSharpCode.NRefactory.CSharp/Formatter/FormattingOptionsFactory.cs b/ICSharpCode.NRefactory.CSharp/Formatter/FormattingOptionsFactory.cs index c430717a4f..6510abf7e9 100644 --- a/ICSharpCode.NRefactory.CSharp/Formatter/FormattingOptionsFactory.cs +++ b/ICSharpCode.NRefactory.CSharp/Formatter/FormattingOptionsFactory.cs @@ -1,4 +1,4 @@ -// +// // FormattingOptionsFactory.cs // // Author: @@ -168,7 +168,7 @@ namespace ICSharpCode.NRefactory.CSharp BlankLinesBeforeUsings = 0, BlankLinesAfterUsings = 1, - + UsingPlacement = UsingPlacement.TopOfFile, BlankLinesBeforeFirstDeclaration = 0, BlankLinesBetweenTypes = 1, diff --git a/ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj b/ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj index 72477e16a5..f3f3ace59f 100644 --- a/ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj +++ b/ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj @@ -267,6 +267,7 @@ + @@ -329,6 +330,7 @@ + diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/AddUsingAction.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/AddUsingAction.cs new file mode 100644 index 0000000000..d43d3895d9 --- /dev/null +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/AddUsingAction.cs @@ -0,0 +1,156 @@ +// Copyright (c) 2013 Daniel Grunwald +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +using System; +using System.Collections.Generic; +using System.Linq; + +using ICSharpCode.NRefactory.CSharp.Resolver; +using ICSharpCode.NRefactory.Semantics; +using ICSharpCode.NRefactory.TypeSystem; + +namespace ICSharpCode.NRefactory.CSharp.Refactoring +{ + /// + /// 1) When a type cannot be resolved, offers to add a using declaration + /// or to replace it with the fully qualified type name. + /// 2) When an extension method cannot be resolved, offers to add a using declaration. + /// 3) When the caret is on a namespace name, offers to add a using declaration + /// and simplify the type references to use the new shorter option. + /// + [ContextAction ("Add using", Description = "Add missing using declaration.")] + public class AddUsingAction : ICodeActionProvider + { + public IEnumerable GetActions(RefactoringContext context) + { + AstNode node = context.GetNode(); + if (node is Identifier) + node = node.Parent; + if (node is SimpleType || node is IdentifierExpression) { + return GetActionsForType(context, node) + .Concat(GetActionsForAddNamespaceUsing(context, node)); + } else if (node is MemberReferenceExpression && node.Parent is InvocationExpression) { + return GetActionsForExtensionMethodInvocation(context, (InvocationExpression)node.Parent); + } else if (node is MemberReferenceExpression) { + return GetActionsForAddNamespaceUsing(context, node); + } else { + return EmptyList.Instance; + } + } + + IEnumerable GetActionsForType(RefactoringContext context, AstNode node) + { + var rr = context.Resolve(node) as UnknownIdentifierResolveResult; + if (rr == null) + return EmptyList.Instance; + + string identifier = rr.Identifier; + int tc = rr.TypeArgumentCount; + string attributeIdentifier = null; + if (node.Parent is Attribute) + attributeIdentifier = identifier + "Attribute"; + + var lookup = new MemberLookup(null, context.Compilation.MainAssembly); + List actions = new List(); + foreach (var typeDefinition in context.Compilation.GetAllTypeDefinitions()) { + if ((typeDefinition.Name == identifier || typeDefinition.Name == attributeIdentifier) + && typeDefinition.TypeParameterCount == tc + && lookup.IsAccessible(typeDefinition, false)) + { + if (typeDefinition.DeclaringTypeDefinition == null) { + actions.Add(NewUsingAction(context, node, typeDefinition.Namespace)); + } + actions.Add(ReplaceWithFullTypeNameAction(context, node, typeDefinition)); + } + } + return actions; + } + + CodeAction NewUsingAction(RefactoringContext context, AstNode node, string ns) + { + return new CodeAction("using " + ns + ";", s => UsingHelper.InsertUsingAndRemoveRedundantNamespaceUsage(context, s, ns)); + } + + CodeAction ReplaceWithFullTypeNameAction(RefactoringContext context, AstNode node, ITypeDefinition typeDefinition) + { + AstType astType = context.CreateShortType(typeDefinition); + string textWithoutGenerics = astType.GetText(); + foreach (var typeArg in node.GetChildrenByRole(Roles.TypeArgument)) { + astType.AddChild(typeArg.Clone(), Roles.TypeArgument); + } + return new CodeAction(textWithoutGenerics, s => s.Replace(node, astType)); + } + + IEnumerable GetActionsForExtensionMethodInvocation(RefactoringContext context, InvocationExpression invocation) + { + var rr = context.Resolve(invocation) as UnknownMethodResolveResult; + if (rr == null) + return EmptyList.Instance; + + var lookup = new MemberLookup(null, context.Compilation.MainAssembly); + HashSet namespaces = new HashSet(); + List result = new List(); + foreach (var typeDefinition in context.Compilation.GetAllTypeDefinitions()) { + if (!(typeDefinition.HasExtensionMethods && lookup.IsAccessible(typeDefinition, false))) { + continue; + } + foreach (var method in typeDefinition.Methods.Where(m => m.IsExtensionMethod && m.Name == rr.MemberName)) { + IType[] inferredTypes; + if (CSharpResolver.IsEligibleExtensionMethod(rr.TargetType, method, true, out inferredTypes)) { + // avoid offering the same namespace twice + if (namespaces.Add(typeDefinition.Namespace)) { + result.Add(NewUsingAction(context, invocation, typeDefinition.Namespace)); + } + break; // continue with the next type + } + } + } + return result; + } + + IEnumerable GetActionsForAddNamespaceUsing(RefactoringContext context, AstNode node) + { + var nrr = context.Resolve(node) as NamespaceResolveResult; + if (nrr == null) + return EmptyList.Instance; + + var trr = context.Resolve(node.Parent) as TypeResolveResult; + if (trr == null) + return EmptyList.Instance; + ITypeDefinition typeDef = trr.Type.GetDefinition(); + if (typeDef == null) + return EmptyList.Instance; + + IList typeArguments; + ParameterizedType parameterizedType = trr.Type as ParameterizedType; + if (parameterizedType != null) + typeArguments = parameterizedType.TypeArguments; + else + typeArguments = EmptyList.Instance; + + var resolver = context.GetResolverStateBefore(node.Parent); + if (resolver.ResolveSimpleName(typeDef.Name, typeArguments) is UnknownIdentifierResolveResult) { + // It's possible to remove the explicit namespace usage and introduce a using instead + return new[] { NewUsingAction(context, node, typeDef.Namespace) }; + } + return EmptyList.Instance; + } + } +} diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/CreateFieldAction.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/CreateFieldAction.cs index d174c8ff2f..c8569765a4 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/CreateFieldAction.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/CreateFieldAction.cs @@ -276,7 +276,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring { var type = GetValidTypes(context.Resolver, expr).ToArray(); var typeInference = new TypeInference(context.Compilation); - typeInference.Algorithm = TypeInferenceAlgorithm.ImprovedReturnAllResults; + typeInference.Algorithm = TypeInferenceAlgorithm.Improved; var inferedType = typeInference.FindTypeInBounds(type, emptyTypes); if (inferedType.Kind == TypeKind.Unknown) return new PrimitiveType("object"); @@ -287,7 +287,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring { var type = GetValidTypes(context.Resolver, expr).ToArray(); var typeInference = new TypeInference(context.Compilation); - typeInference.Algorithm = TypeInferenceAlgorithm.ImprovedReturnAllResults; + typeInference.Algorithm = TypeInferenceAlgorithm.Improved; var inferedType = typeInference.FindTypeInBounds(type, emptyTypes); return inferedType; } diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/SortUsingsAction.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/SortUsingsAction.cs index 12c15c765c..371f621b60 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/SortUsingsAction.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/SortUsingsAction.cs @@ -23,7 +23,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring foreach (var block in blocks) { var originalNodes = block.ToArray(); - var sortedNodes = SortUsingBlock(originalNodes, context).ToArray(); + var sortedNodes = UsingHelper.SortUsingBlock(originalNodes, context).ToArray(); for (var i = 0; i < originalNodes.Length; ++i) script.Replace(originalNodes[i], sortedNodes[i].Clone()); @@ -68,78 +68,5 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring for (var node = firstNode; IsUsingDeclaration(node); node = node.NextSibling) yield return node; } - - private static IEnumerable SortUsingBlock(IEnumerable nodes, RefactoringContext context) - { - var infos = nodes.Select(_ => new UsingInfo(_, context)); - var orderedInfos = infos.OrderBy(_ => _, new UsingInfoComparer()); - var orderedNodes = orderedInfos.Select(_ => _.Node); - - return orderedNodes; - } - - - private sealed class UsingInfo - { - public AstNode Node { get; private set; } - - public string Alias { get; private set; } - public string Name { get; private set; } - - public bool IsAlias { get; private set; } - public bool IsAssembly { get; private set; } - public bool IsSystem { get; private set; } - - public UsingInfo(AstNode node, RefactoringContext context) - { - var importAndAlias = GetImportAndAlias(node); - - Node = node; - - Alias = importAndAlias.Item2; - Name = importAndAlias.Item1.ToString(); - - IsAlias = Alias != null; - - var result = context.Resolve(importAndAlias.Item1) as NamespaceResolveResult; - var mainSourceAssembly = result != null ? result.Namespace.ContributingAssemblies.First() : null; - var unresolvedAssembly = mainSourceAssembly != null ? mainSourceAssembly.UnresolvedAssembly : null; - IsAssembly = unresolvedAssembly is DefaultUnresolvedAssembly; - - IsSystem = IsAssembly && Name.StartsWith("System"); - } - - private static Tuple GetImportAndAlias(AstNode node) - { - var plainUsing = node as UsingDeclaration; - if (plainUsing != null) - return Tuple.Create(plainUsing.Import, (string)null); - - var aliasUsing = node as UsingAliasDeclaration; - if (aliasUsing != null) - return Tuple.Create(aliasUsing.Import, aliasUsing.Alias); - - throw new InvalidOperationException(string.Format("Invalid using node: {0}", node)); - } - } - - private sealed class UsingInfoComparer: IComparer - { - public int Compare(UsingInfo x, UsingInfo y) - { - if (x.IsAlias != y.IsAlias) - return x.IsAlias && !y.IsAlias ? 1 : -1; - else if (x.IsAssembly != y.IsAssembly) - return x.IsAssembly && !y.IsAssembly ? -1 : 1; - else if (x.IsSystem != y.IsSystem) - return x.IsSystem && !y.IsSystem ? -1 : 1; - else if (x.Alias != y.Alias) - return Comparer.Default.Compare(x.Alias, y.Alias); - else if (x.Name != y.Name) - return Comparer.Default.Compare(x.Name, y.Name); - else - return 0; - } - } } } diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssue.cs index 2ae89cb6e5..9394887a25 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssue.cs @@ -1,4 +1,4 @@ -// +// // InspectionIssue.cs // // Author: diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ExceptionRethrowIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ExceptionRethrowIssue.cs index c822e003a2..27528ae92a 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ExceptionRethrowIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ExceptionRethrowIssue.cs @@ -63,7 +63,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring var action = new CodeAction(ctx.TranslateString("Change to 'throw;'"), script => { script.Replace(localThrowStatement, new ThrowStatement()); }); - AddIssue(localThrowStatement, title, new [] { action }); + AddIssue(localThrowStatement, title, action); } } } diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/GatherVisitorBase.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/GatherVisitorBase.cs index 61819670a5..864abdda32 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/GatherVisitorBase.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/GatherVisitorBase.cs @@ -1,4 +1,4 @@ -// +// // GatherVisitorBase.cs // // Author: @@ -78,6 +78,16 @@ namespace ICSharpCode.NRefactory.CSharp FoundIssues.Add(new CodeIssue(title, start, end, fix != null ? new CodeAction(title, fix) : null)); } + protected void AddIssue(AstNode node, string title, CodeAction fix) + { + FoundIssues.Add(new CodeIssue(title, node.StartLocation, node.EndLocation, fix)); + } + + protected void AddIssue(TextLocation start, TextLocation end, string title, CodeAction fix) + { + FoundIssues.Add(new CodeIssue (title, start, end, fix)); + } + protected void AddIssue(AstNode node, string title, IEnumerable fixes) { FoundIssues.Add(new CodeIssue(title, node.StartLocation, node.EndLocation, fixes)); @@ -87,10 +97,5 @@ namespace ICSharpCode.NRefactory.CSharp { FoundIssues.Add(new CodeIssue (title, start, end, fixes)); } - - } - - } - diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/IncorrectExceptionParameterOrderingIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/IncorrectExceptionParameterOrderingIssue.cs index 73e5051f52..3e61e984ae 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/IncorrectExceptionParameterOrderingIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/IncorrectExceptionParameterOrderingIssue.cs @@ -78,13 +78,13 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring if (!func(leftLength, rightLength)) AddIssue(objectCreateExpression, context.TranslateString("The parameters are in the wrong order"), - GetActions(objectCreateExpression, firstParam, secondParam)); + GetAction(objectCreateExpression, firstParam, secondParam)); } - IEnumerable GetActions(ObjectCreateExpression objectCreateExpression, + CodeAction GetAction(ObjectCreateExpression objectCreateExpression, PrimitiveExpression firstParam, PrimitiveExpression secondParam) { - yield return new CodeAction(context.TranslateString("Swap parameters"), script => { + return new CodeAction(context.TranslateString("Swap parameters"), script => { var newOCE = objectCreateExpression.Clone() as ObjectCreateExpression; newOCE.Arguments.Clear(); newOCE.Arguments.Add(secondParam.Clone()); diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ReferenceToStaticMemberViaDerivedTypeIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ReferenceToStaticMemberViaDerivedTypeIssue.cs index 5342de7b4d..7ebd2d598f 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ReferenceToStaticMemberViaDerivedTypeIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ReferenceToStaticMemberViaDerivedTypeIssue.cs @@ -96,16 +96,16 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring if (v.IsContained) return; AddIssue(issueAnchor, context.TranslateString("Static method invoked via derived type"), - GetActions(context, targetExpression, member)); + GetAction(context, targetExpression, member)); } - IEnumerable GetActions(BaseRefactoringContext context, Expression targetExpression, + CodeAction GetAction(BaseRefactoringContext context, Expression targetExpression, IMember member) { var builder = context.CreateTypeSytemAstBuilder(targetExpression); var newType = builder.ConvertType(member.DeclaringType); string description = string.Format("{0} '{1}'", context.TranslateString("Use base class"), newType.GetText()); - yield return new CodeAction(description, script => { + return new CodeAction(description, script => { script.Replace(targetExpression, newType); }); } diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ResultOfAsyncCallShouldNotBeIgnoredIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ResultOfAsyncCallShouldNotBeIgnoredIssue.cs index d4a40e3f2b..c42368efaa 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ResultOfAsyncCallShouldNotBeIgnoredIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ResultOfAsyncCallShouldNotBeIgnoredIssue.cs @@ -50,7 +50,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring return; var rr = ctx.Resolve(invocation) as InvocationResolveResult; if (rr != null && (rr.Type.IsKnownType(KnownTypeCode.Task) || rr.Type.IsKnownType(KnownTypeCode.TaskOfT))) { - AddIssue(invocation, "Exceptions in async call will be silently ignored because the returned task is unused"); + AddIssue(invocation, ctx.TranslateString("Exceptions in async call will be silently ignored because the returned task is unused")); } } } diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/Script.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/Script.cs index 75d92c1c7e..307285456d 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/Script.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/Script.cs @@ -143,38 +143,38 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring get { return options; } } - public void InsertBefore(AstNode node, AstNode insertNode) + public void InsertBefore(AstNode node, AstNode newNode) { var startOffset = GetCurrentOffset(new TextLocation(node.StartLocation.Line, 1)); - var output = OutputNode (GetIndentLevelAt (startOffset), insertNode); + var output = OutputNode (GetIndentLevelAt (startOffset), newNode); string text = output.Text; - if (!(insertNode is Expression || insertNode is AstType)) + if (!(newNode is Expression || newNode is AstType)) text += Options.EolMarker; InsertText(startOffset, text); output.RegisterTrackedSegments(this, startOffset); - CorrectFormatting (node, insertNode); + CorrectFormatting (node, newNode); } - public void InsertAfter(AstNode node, AstNode insertNode) + public void InsertAfter(AstNode node, AstNode newNode) { var indentOffset = GetCurrentOffset(new TextLocation(node.StartLocation.Line, 1)); - var output = OutputNode (GetIndentLevelAt (indentOffset), insertNode); + var output = OutputNode (GetIndentLevelAt (indentOffset), newNode); string text = output.Text; - if (!(insertNode is Expression || insertNode is AstType)) + if (!(newNode is Expression || newNode is AstType)) text = Options.EolMarker + text; var insertOffset = GetCurrentOffset(node.EndLocation); InsertText(insertOffset, text); output.RegisterTrackedSegments(this, insertOffset); - CorrectFormatting (node, insertNode); + CorrectFormatting (node, newNode); } - public void AddTo(BlockStatement bodyStatement, AstNode insertNode) + public void AddTo(BlockStatement bodyStatement, AstNode newNode) { var startOffset = GetCurrentOffset(bodyStatement.LBraceToken.EndLocation); - var output = OutputNode(1 + GetIndentLevelAt(startOffset), insertNode, true); + var output = OutputNode(1 + GetIndentLevelAt(startOffset), newNode, true); InsertText(startOffset, output.Text); output.RegisterTrackedSegments(this, startOffset); - CorrectFormatting (null, insertNode); + CorrectFormatting (null, newNode); } public virtual Task Link (params AstNode[] nodes) diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/UsingHelper.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/UsingHelper.cs new file mode 100644 index 0000000000..924bffd769 --- /dev/null +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/UsingHelper.cs @@ -0,0 +1,180 @@ +// Copyright (c) AlphaSierraPapa for the SharpDevelop Team +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of this +// software and associated documentation files (the "Software"), to deal in the Software +// without restriction, including without limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons +// to whom the Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR +// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE +// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.Linq; +using ICSharpCode.NRefactory.CSharp.Resolver; +using ICSharpCode.NRefactory.Semantics; + +namespace ICSharpCode.NRefactory.CSharp.Refactoring +{ + /// + /// Helper methods for managing using declarations. + /// + public class UsingHelper + { + /// + /// Inserts 'using ns;' in the current scope, and then removes all explicit + /// usages of ns that were made redundant by the new using. + /// + public static void InsertUsingAndRemoveRedundantNamespaceUsage(RefactoringContext context, Script script, string ns) + { + InsertUsing(context, script, new UsingDeclaration(ns)); + // TODO: remove the usages that were made redundant + } + + /// + /// Inserts 'newUsing' in the current scope. + /// This method will try to insert new usings in the correct position (depending on + /// where the existing usings are; and maintaining the sort order). + /// + public static void InsertUsing(RefactoringContext context, Script script, AstNode newUsing) + { + UsingInfo newUsingInfo = new UsingInfo(newUsing, context); + AstNode enclosingNamespace = context.GetNode() ?? context.RootNode; + // Find nearest enclosing parent that has usings: + AstNode usingParent = enclosingNamespace; + while (usingParent != null && !usingParent.Children.OfType().Any()) + usingParent = usingParent.Parent; + if (usingParent == null) { + // No existing usings at all -> use the default location + if (script.FormattingOptions.UsingPlacement == UsingPlacement.TopOfFile) { + usingParent = context.RootNode; + } else { + usingParent = enclosingNamespace; + } + } + // Find the main block of using declarations in the chosen scope: + AstNode blockStart = usingParent.Children.FirstOrDefault(IsUsingDeclaration); + AstNode insertionPoint; + if (blockStart == null) { + // no using declarations in the file + Debug.Assert(SyntaxTree.MemberRole == NamespaceDeclaration.MemberRole); + insertionPoint = usingParent.GetChildrenByRole(SyntaxTree.MemberRole).SkipWhile(CanAppearBeforeUsings).FirstOrDefault(); + } else { + insertionPoint = blockStart; + while (IsUsingDeclaration(insertionPoint) && newUsingInfo.CompareTo(new UsingInfo(insertionPoint, context)) > 0) + insertionPoint = insertionPoint.NextSibling; + } + if (insertionPoint != null) { + script.InsertBefore(insertionPoint, newUsing); + } + } + + static bool IsUsingDeclaration(AstNode node) + { + return node is UsingDeclaration || node is UsingAliasDeclaration; + } + + static bool CanAppearBeforeUsings(AstNode node) + { + if (node is ExternAliasDeclaration) + return true; + if (node is PreProcessorDirective) + return true; + Comment c = node as Comment; + if (c != null) + return !c.IsDocumentation; + return false; + } + + /// + /// Sorts the specified usings. + /// + public static IEnumerable SortUsingBlock(IEnumerable nodes, BaseRefactoringContext context) + { + var infos = nodes.Select(_ => new UsingInfo(_, context)); + var orderedInfos = infos.OrderBy(_ => _); + var orderedNodes = orderedInfos.Select(_ => _.Node); + + return orderedNodes; + } + + + private sealed class UsingInfo : IComparable + { + public AstNode Node; + + public string Alias; + public string Name; + + public bool IsAlias; + public bool HasTypesFromOtherAssemblies; + public bool IsSystem; + + public UsingInfo(AstNode node, BaseRefactoringContext context) + { + var importAndAlias = GetImportAndAlias(node); + + Node = node; + + Alias = importAndAlias.Item2; + Name = importAndAlias.Item1.ToString(); + + IsAlias = Alias != null; + + ResolveResult rr; + if (node.Ancestors.Contains(context.RootNode)) { + rr = context.Resolve(importAndAlias.Item1); + } else { + // It's possible that we're looking at a new using that + // isn't part of the AST. + var resolver = new CSharpAstResolver(new CSharpResolver(context.Compilation), node); + rr = resolver.Resolve(importAndAlias.Item1); + } + + var nrr = rr as NamespaceResolveResult; + HasTypesFromOtherAssemblies = nrr != null && nrr.Namespace.ContributingAssemblies.Any(a => !a.IsMainAssembly); + + IsSystem = HasTypesFromOtherAssemblies && (Name == "System" || Name.StartsWith("System.", StringComparison.Ordinal)); + } + + private static Tuple GetImportAndAlias(AstNode node) + { + var plainUsing = node as UsingDeclaration; + if (plainUsing != null) + return Tuple.Create(plainUsing.Import, (string)null); + + var aliasUsing = node as UsingAliasDeclaration; + if (aliasUsing != null) + return Tuple.Create(aliasUsing.Import, aliasUsing.Alias); + + throw new InvalidOperationException(string.Format("Invalid using node: {0}", node)); + } + + public int CompareTo(UsingInfo y) + { + UsingInfo x = this; + if (x.IsAlias != y.IsAlias) + return x.IsAlias ? 1 : -1; + else if (x.HasTypesFromOtherAssemblies != y.HasTypesFromOtherAssemblies) + return x.HasTypesFromOtherAssemblies ? -1 : 1; + else if (x.IsSystem != y.IsSystem) + return x.IsSystem ? -1 : 1; + else if (x.Alias != y.Alias) + return StringComparer.Ordinal.Compare(x.Alias, y.Alias); + else if (x.Name != y.Name) + return StringComparer.Ordinal.Compare(x.Name, y.Name); + else + return 0; + } + } + } +} diff --git a/ICSharpCode.NRefactory.CSharp/TypeSystem/TypeSystemConvertVisitor.cs b/ICSharpCode.NRefactory.CSharp/TypeSystem/TypeSystemConvertVisitor.cs index d6c0cd5a34..735b1eeea7 100644 --- a/ICSharpCode.NRefactory.CSharp/TypeSystem/TypeSystemConvertVisitor.cs +++ b/ICSharpCode.NRefactory.CSharp/TypeSystem/TypeSystemConvertVisitor.cs @@ -1178,7 +1178,7 @@ namespace ICSharpCode.NRefactory.CSharp.TypeSystem // traverse AST backwards until the next non-whitespace node for (AstNode node = entityDeclaration.PrevSibling; node != null && node.NodeType == NodeType.Whitespace; node = node.PrevSibling) { Comment c = node as Comment; - if (c != null && (c.CommentType == CommentType.Documentation || c.CommentType == CommentType.MultiLineDocumentation)) { + if (c != null && c.IsDocumentation) { if (documentation == null) documentation = new List(); if (c.CommentType == CommentType.MultiLineDocumentation) { diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/AddUsing/AddUsingActionAlphabeticalTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/AddUsing/AddUsingActionAlphabeticalTests.cs new file mode 100644 index 0000000000..0422a373df --- /dev/null +++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/AddUsing/AddUsingActionAlphabeticalTests.cs @@ -0,0 +1,229 @@ +using NUnit.Framework; +using ICSharpCode.NRefactory.CSharp.CodeIssues; +using ICSharpCode.NRefactory.CSharp.Refactoring; +using ICSharpCode.NRefactory.CSharp.CodeActions; +using ICSharpCode.NRefactory.CSharp; +using System.Linq; + +namespace ICSharpCode.NRefactory.CSharp.CodeIssues.UnresolvedType +{ + [TestFixture] + public class UnresolvedTypeActionAlphabeticalTests : ContextActionTestBase + { + [Test] + public void ShouldAddUsingAtStartIfItIsTheFirstAlphabetically() + { + string testCode = +@"namespace OuterNamespace +{ + using System.IO; + + class TestClass + { + private $List writerList; + } +}"; + + string expectedOutput = +@"namespace OuterNamespace +{ + using System.Collections.Generic; + using System.IO; + + class TestClass + { + private List writerList; + } +}"; + + Test(new AddUsingAction(), testCode, expectedOutput); + } + + [Test] + public void ShouldInsertUsingBetweenExistingUsings() + { + string testCode = +@"namespace OuterNamespace +{ + using System; + using System.IO; + + class TestClass + { + private $List writerList; + } +}"; + + string expectedOutput = +@"namespace OuterNamespace +{ + using System; + using System.Collections.Generic; + using System.IO; + + class TestClass + { + private List writerList; + } +}"; + + Test(new AddUsingAction(), testCode, expectedOutput); + } + + [Test] + [Ignore("Add using does not honor the blank line setting yet")] + public void ShouldInsertUsingAfterExistingUsings() + { + string testCode = +@"namespace OuterNamespace +{ + using System; + using System.Collections.Generic; + + class TestClass + { + private List<$TextWriter> writerList; + } +}"; + + string expectedOutput = +@"namespace OuterNamespace +{ + using System; + using System.Collections.Generic; + using System.IO; + + class TestClass + { + private List writerList; + } +}"; + + Test(new AddUsingAction(), testCode, expectedOutput); + } + + [Test] + [Ignore("Add using does not honor the blank line setting yet")] + public void ShouldAddBlankLinesAfterUsingsWhenAddingAtEnd() + { + string testCode = +@"namespace OuterNamespace +{ + using System; + using System.Collections.Generic; + class TestClass + { + private List<$TextWriter> writerList; + } +}"; + + string expectedOutput = +@"namespace OuterNamespace +{ + using System; + using System.Collections.Generic; + using System.IO; + + class TestClass + { + private List writerList; + } +}"; + + Test(new AddUsingAction(), testCode, expectedOutput); + } + + #region System Namespaces + + [Test] + public void ShouldBeAbleToPlaceSystemNamespacesFirst() + { + string testCode = +@"namespace OuterNamespace +{ + using ANamespace; + + class TestClass + { + private $TextWriter writer; + } +}"; + + string expectedOutput = +@"namespace OuterNamespace +{ + using System.IO; + using ANamespace; + + class TestClass + { + private TextWriter writer; + } +}"; + Test(new AddUsingAction(), testCode, expectedOutput); + } + + [Test] + public void ShouldNotPlaceNonSystemNamespacesStartingWithSystemFirst() + { + string testCode = +@"namespace A { class B { } } +namespace OuterNamespace +{ + using SystemA; + + class TestClass + { + private $B b; + } +}"; + + string expectedOutput = +@"namespace A { class B { } } +namespace OuterNamespace +{ + using A; + using SystemA; + + class TestClass + { + private B b; + } +}"; + + Test(new AddUsingAction(), testCode, expectedOutput); + } + + [Test] + public void ShouldPlaceSystemBeforeOtherNamespaces() + { + string testCode = +@"namespace OuterNamespace +{ + using System.Collections.Generic; + + class TestClass + { + private List<$DateTime> dates; + } +}"; + + string expectedOutput = +@"namespace OuterNamespace +{ + using System; + using System.Collections.Generic; + + class TestClass + { + private List dates; + } +}"; + + Test(new AddUsingAction(), testCode, expectedOutput); + } + + #endregion + } +} + diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/AddUsing/AddUsingActionInsideNamespaceTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/AddUsing/AddUsingActionInsideNamespaceTests.cs new file mode 100644 index 0000000000..d30c2fceb8 --- /dev/null +++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/AddUsing/AddUsingActionInsideNamespaceTests.cs @@ -0,0 +1,203 @@ +using NUnit.Framework; +using ICSharpCode.NRefactory.CSharp.CodeIssues; +using ICSharpCode.NRefactory.CSharp.CodeActions; +using ICSharpCode.NRefactory.CSharp.Refactoring; +using System.Linq; + +namespace ICSharpCode.NRefactory.CSharp.CodeIssues.UnresolvedType +{ + [TestFixture] + public class UnresolvedTypeActionInsideNamespaceTests : ContextActionTestBase + { + [Test] + public void ShouldInsertUsingStatement() + { + string testCode = +@"namespace TestNamespace +{ + class TestClass + { + private $List stringList; + } +}"; + + string expectedOutput = +@"namespace TestNamespace +{ + using System.Collections.Generic; + class TestClass + { + private List stringList; + } +}"; + + formattingOptions.UsingPlacement = UsingPlacement.InsideNamespace; + formattingOptions.BlankLinesAfterUsings = 0; + Test(new AddUsingAction(), testCode, expectedOutput); + } + + [Test] + [Ignore("Add using does not honor the blank line setting yet")] + public void ShouldAddBlankLinesBeforeUsingStatement() + { + string testCode = +@"namespace TestNamespace +{ + class TestClass + { + private $List stringList; + } +}"; + + string expectedOutput = +@"namespace TestNamespace +{ + + + using System.Collections.Generic; + class TestClass + { + private List stringList; + } +}"; + + formattingOptions.UsingPlacement = UsingPlacement.InsideNamespace; + formattingOptions.BlankLinesBeforeUsings = 2; + formattingOptions.BlankLinesAfterUsings = 0; + Test(new AddUsingAction(), testCode, expectedOutput); + } + + [Test] + [Ignore("Add using does not honor the blank line setting yet")] + public void ShouldAddBlankLinesAfterUsingStatements() + { + string testCode = +@"namespace TestNamespace +{ + class TestClass + { + private $List stringList; + } +}"; + + string expectedOutput = +@"namespace TestNamespace +{ + using System.Collections.Generic; + + + class TestClass + { + private List stringList; + } +}"; + + formattingOptions.UsingPlacement = UsingPlacement.InsideNamespace; + formattingOptions.BlankLinesAfterUsings = 2; + Test(new AddUsingAction(), testCode, expectedOutput); + } + + [Test] + [Ignore("Something is wrong with the blank lines")] + public void ShouldAddUsingAfterExistingUsings() + { + string testCode = +@"namespace TestNamespace +{ + using System; + + class TestClass + { + private $List stringList; + } +}"; + + string expectedOutput = +@"namespace TestNamespace +{ + using System; + using System.Collections.Generic; + + class TestClass + { + private List stringList; + } +}"; + + formattingOptions.UsingPlacement = UsingPlacement.InsideNamespace; + Test(new AddUsingAction(), testCode, expectedOutput); + } + + [Test] + [Ignore("Add using does not honor the blank line setting yet")] + public void ShouldAddUsingInMostNestedNamespace() + { + string testCode = +@"namespace OuterNamespace +{ + namespace InnerNamespace + { + class TestClass + { + private $List stringList; + } + } +}"; + + string expectedOutput = +@"namespace OuterNamespace +{ + namespace InnerNamespace + { + using System.Collections.Generic; + + class TestClass + { + private List stringList; + } + } +}"; + + formattingOptions.UsingPlacement = UsingPlacement.InsideNamespace; + Test(new AddUsingAction(), testCode, expectedOutput); + } + + [Test] + [Ignore("Something is wrong with the blank lines")] + public void ShouldAddUsingAfterExistingUsingsInMostNestedNamespace() + { + string testCode = +@"namespace OuterNamespace +{ + namespace InnerNamespace + { + using System; + + class TestClass + { + private $List stringList; + } + } +}"; + + string expectedOutput = +@"namespace OuterNamespace +{ + namespace InnerNamespace + { + using System; + using System.Collections.Generic; + + class TestClass + { + private List stringList; + } + } +}"; + + formattingOptions.UsingPlacement = UsingPlacement.InsideNamespace; + Test(new AddUsingAction(), testCode, expectedOutput); + } + } +} + diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/AddUsing/AddUsingActionTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/AddUsing/AddUsingActionTests.cs new file mode 100644 index 0000000000..02f1c389c5 --- /dev/null +++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/AddUsing/AddUsingActionTests.cs @@ -0,0 +1,320 @@ +using ICSharpCode.NRefactory.CSharp.CodeIssues; +using NUnit.Framework; +using ICSharpCode.NRefactory.CSharp.CodeActions; +using ICSharpCode.NRefactory.CSharp.Refactoring; +using System.Linq; + +namespace ICSharpCode.NRefactory.CSharp.CodeIssues.UnresolvedType +{ + [TestFixture] + public class UnresolvedTypeIssueTests : ContextActionTestBase + { + void UnresolvedTypeName(string code, string typeName, params string[] namespaces) + { + TestActionDescriptions( + new AddUsingAction(), code, + namespaces.SelectMany(ns => new[] { + "using " + ns + ";", + ns + "." + typeName + }).ToArray()); + } + + #region Field Declarations + [Test] + public void ShouldReturnAnIssueForUnresolvedFieldDeclarations() + { + UnresolvedTypeName(@"class Foo { + private $TextWriter textWriter; +}", "TextWriter", "System.IO"); + } + + [Test] + public void ShouldNotReturnAnyIssuesIfFieldTypeIsResolved() + { + TestWrongContext(@"using System.IO; +class Foo { + private $TextWriter textWriter; +}"); + } + + [Test] + public void ShouldReturnAnIssueIfFieldTypeArgumentIsNotResolvable() + { + UnresolvedTypeName( + @"using System.Collections.Generic; + +class Foo +{ + private List<$AttributeTargets> targets; +}", "AttributeTargets", "System"); + } + + [Test] + public void ShouldNotReturnAnIssueIfFieldTypeArgumentIsResolvable() + { + TestWrongContext( + @"using System; +using System.Collections.Generic; + +class Foo +{ + private List<$AttributeTargets> notifiers; +}"); + } + + [Test] + public void ShouldNotReturnAnIssueIfFieldTypeArgumentIsPrimitiveType() + { + TestWrongContext( + @"using System.Collections.Generic; + +class Foo +{ + private List<$string> notifiers; +}"); + } + #endregion + + #region Method Return Types + [Test] + public void ShouldReturnIssueForUnresolvedReturnType() + { + UnresolvedTypeName( + @"class Foo +{ + $TextWriter Bar () + { + return null; + } +}", "TextWriter", "System.IO"); + } + + [Test] + public void ShouldNotReturnIssueForResolvedReturnType() + { + TestWrongContext( + @"using System.IO; + +class Foo +{ + $TextWriter Bar () + { + return null; + } +}"); + } + #endregion + + #region Local Variables + [Test] + public void ShouldReturnIssueForUnresolvedLocalVariableDeclaration() + { + UnresolvedTypeName( + @"class Foo +{ + void Bar () + { + $TextWriter writer; + } +}", "TextWriter", "System.IO"); + } + + [Test] + public void ShouldNotReturnIssueForResolvedLocalVariableDeclaration() + { + TestWrongContext( + @"using System.IO; + +class Foo +{ + void Bar () + { + $TextWriter writer; + } +}"); + } + #endregion + + #region Method Parameters + [Test] + public void ShouldReturnIssueIfMethodParameterIsNotResolvable() + { + UnresolvedTypeName( + @"class Foo +{ + void Bar ($TextWriter writer) + { + } +}", "TextWriter", "System.IO"); + } + + [Test] + public void ShouldNotReturnAnIssueIfMethodParameterIsResolvable() + { + TestWrongContext( + @"using System.IO; + +class Foo +{ + void Bar ($TextWriter writer) + { + } +}"); + } + #endregion + + #region Base Types + [Test] + public void ShouldReturnIssueIfBaseClassIsNotResolvable() + { + UnresolvedTypeName( + @"class Foo : $List +{ +}", "List<>", "System.Collections.Generic"); + } + + [Test] + public void ShouldNotReturnIssueIfBaseClassIsResolvable() + { + TestWrongContext( + @"using System.Collections.Generic; + +class Foo : $List +{ +}"); + } + + [Test] + public void ShouldReturnIssueIfGenericInterfaceIsMissingButNonGenericIsPresent() + { + UnresolvedTypeName( + @"using System.Collections; +class Foo : $IEnumerable +{ +}", "IEnumerable<>", "System.Collections.Generic"); + } + + [Test] + public void ShouldReturnIssueIfNonGenericInterfaceIsMissingButGenericIsPresent() + { + UnresolvedTypeName( + @"using System.Collections.Generic; +class Foo : $IEnumerable +{ +}", "IEnumerable", "System.Collections"); + } + + #endregion + + #region Member Access + [Test] + public void ShouldReturnIssueIfEnumValueIsNotResolvable() + { + UnresolvedTypeName( + @"class Foo +{ + void Bar () + { + var support = $AttributeTargets.Assembly; + } +}", "AttributeTargets", "System"); + } + + [Test] + public void ShouldNotReturnIssueIfEnumValueIsResolvable() + { + TestWrongContext( + @"using System; +class Foo +{ + void Bar () + { + var support = $AttributeTargets.Assembly; + } +}"); + } + #endregion + + [Test] + public void ShouldReturnIssueIfAttributeIsNotResolvable() + { + UnresolvedTypeName( + @"[$Serializable] +class Foo +{ +}", "SerializableAttribute", "System"); + } + + [Test] + public void ShouldNotReturnIssueIfAttributeIsResolvable() + { + TestWrongContext( + @"using System; + +[$Serializable] +class Foo +{ +}"); + } + + [Test] + public void ShouldReturnIssueIfTypeArgumentIsNotResolvable() + { + UnresolvedTypeName( + @"using System.Collections.Generic; + +class Test +{ + void TestMethod() + { + var list = new List<$Stream>(); + } +}", "Stream", "System.IO"); + } + + [Test] + public void ShouldReturnIssueForUnresolvedExtensionMethod() + { + TestActionDescriptions( + new AddUsingAction(), + @"using System.Collections.Generic; + +class Test +{ + void TestMethod() + { + var list = new List(); + var first = list.$First(); + } +}", "using System.Linq;"); + } + + [Test] + public void ShouldReturnMultipleNamespaceSuggestions() + { + UnresolvedTypeName( + @"namespace A { public class TestClass { } } +namespace B { public class TestClass { } } +namespace C +{ + public class Test + { + private $TestClass testClass; + } +}", "TestClass", "A", "B"); + } + + [Test] + public void InnerTypeCanOnlyBeReferredToByFullName() + { + TestActionDescriptions( + new AddUsingAction(), + @"class Outer { public class Inner {} } +public class Test +{ + private $Inner t; +} +", "Outer.Inner"); + } + } +} diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/AddUsing/AddUsingRunActionTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/AddUsing/AddUsingRunActionTests.cs new file mode 100644 index 0000000000..c4820c42bc --- /dev/null +++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/AddUsing/AddUsingRunActionTests.cs @@ -0,0 +1,248 @@ +using NUnit.Framework; +using ICSharpCode.NRefactory.CSharp.CodeIssues; +using ICSharpCode.NRefactory.CSharp.CodeActions; +using ICSharpCode.NRefactory.CSharp.Refactoring; +using System.Linq; + +namespace ICSharpCode.NRefactory.CSharp.CodeIssues.UnresolvedType +{ + [TestFixture] + public class UnresolvedTypeActionTests : ContextActionTestBase + { + [Test] + [Ignore("Add using does not honor the blank line setting yet")] + public void ShouldInsertUsingStatement() + { + string testCode = +@"namespace TestNamespace +{ + class TestClass + { + private $List stringList; + } +}"; + + string expectedOutput = +@"using System.Collections.Generic; + +namespace TestNamespace +{ + class TestClass + { + private List stringList; + } +}"; + + Test(new AddUsingAction(), testCode, expectedOutput); + } + + [Test] + [Ignore("Add using does not honor the blank line setting yet")] + public void ShouldAddBlankLinesAfterUsings() + { + string testCode = +@"namespace TestNamespace +{ + class TestClass + { + private $List stringList; + } +}"; + + string expectedOutput = +@"using System.Collections.Generic; + + +namespace TestNamespace +{ + class TestClass + { + private List stringList; + } +}"; + + formattingOptions.BlankLinesAfterUsings = 2; + Test(new AddUsingAction(), testCode, expectedOutput); + } + + [Test] + [Ignore("Add using does not honor the blank line setting yet")] + public void ShouldAddBlankLinesBeforeUsing() + { + string testCode = +@"namespace TestNamespace +{ + class TestClass + { + private $List stringList; + } +}"; + + string expectedOutput = +@" + +using System.Collections.Generic; + +namespace TestNamespace +{ + class TestClass + { + private List stringList; + } +}"; + + formattingOptions.BlankLinesBeforeUsings = 2; + Test(new AddUsingAction(), testCode, expectedOutput); + } + + [Test] + [Ignore("Add using does not honor the blank line setting yet")] + public void ShouldAddAfterExistingUsingStatements() + { + string testCode = +@"using System; +namespace TestNamespace +{ + class TestClass + { + private $List stringList; + } +}"; + + string expectedOutput = +@"using System; +using System.Collections.Generic; + +namespace TestNamespace +{ + class TestClass + { + private List stringList; + } +}"; + + Test(new AddUsingAction(), testCode, expectedOutput); + } + + [Test] + [Ignore("Something else is broken regarding blank lines as well")] + public void ShouldNotAddBlankLinesAfterIfTheyAreAlreadyThere() + { + string testCode = +@"using System; + +namespace TestNamespace +{ + class TestClass + { + private $List stringList; + } +}"; + + string expectedOutput = +@"using System; +using System.Collections.Generic; + +namespace TestNamespace +{ + class TestClass + { + private List stringList; + } +}"; + + Test(new AddUsingAction(), testCode, expectedOutput); + } + + [Test] + [Ignore("Something else is broken regarding blank lines as well")] + public void ShouldLeaveAdditionalBlankLinesThatAlreadyExist() + { + string testCode = +@"using System; + + +namespace TestNamespace +{ + class TestClass + { + private $List stringList; + } +}"; + + string expectedOutput = +@"using System; +using System.Collections.Generic; + + +namespace TestNamespace +{ + class TestClass + { + private List stringList; + } +}"; + + Test(new AddUsingAction(), testCode, expectedOutput); + } + + [Test] + [Ignore("Add using does not honor the blank line setting yet")] + public void ShouldAddFirstUsingAfterComments() + { + string testCode = +@"// This is the file header. +// It contains any copyright information. +namespace TestNamespace +{ + class TestClass + { + private $List stringList; + } +}"; + + string expectedOutput = +@"// This is the file header. +// It contains any copyright information. +using System.Collections.Generic; + +namespace TestNamespace +{ + class TestClass + { + private List stringList; + } +}"; + + Test(new AddUsingAction(), testCode, expectedOutput); + } + + [Test] + [Ignore("Add using does not honor the blank line setting yet")] + public void ShouldBeAbleToFixAttributeWithShortName() + { + string testCode = +@"namespace TestNamespace +{ + [$Serializable] + class TestClass + { + } +}"; + + string expectedOutput = +@"using System; + +namespace TestNamespace +{ + [Serializable] + class TestClass + { + } +}"; + + Test(new AddUsingAction(), testCode, expectedOutput); + } + } +} + diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ContextActionTestBase.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ContextActionTestBase.cs index 5fc59f8809..8ebd281f79 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ContextActionTestBase.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ContextActionTestBase.cs @@ -1,6 +1,6 @@ // // ContextActionTestBase.cs -// +// // Author: // Mike Krüger // @@ -35,6 +35,14 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions { public abstract class ContextActionTestBase { + protected CSharpFormattingOptions formattingOptions; + + [SetUp] + public virtual void SetUp() + { + formattingOptions = FormattingOptionsFactory.CreateMono(); + } + internal static string HomogenizeEol (string str) { var sb = new StringBuilder (); @@ -53,10 +61,15 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions return sb.ToString (); } - public void Test (string input, string output, int action = 0, bool expectErrors = false) + public void Test (string input, string output, int action = 0, bool expectErrors = false) where T : ICodeActionProvider, new () { - string result = RunContextAction (new T (), HomogenizeEol (input), action, expectErrors); + Test(new T(), input, output, action, expectErrors); + } + + public void Test (ICodeActionProvider provider, string input, string output, int action = 0, bool expectErrors = false) + { + string result = RunContextAction (provider, HomogenizeEol (input), action, expectErrors); bool passed = result == output; if (!passed) { Console.WriteLine ("-----------Expected:"); @@ -65,13 +78,13 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions Console.WriteLine (result); } Assert.AreEqual (HomogenizeEol (output), result); - } - + } - protected static string RunContextAction (ICodeActionProvider action, string input, + protected string RunContextAction (ICodeActionProvider action, string input, int actionIndex = 0, bool expectErrors = false) { var context = TestRefactoringContext.Create (input, expectErrors); + context.FormattingOptions = formattingOptions; bool isValid = action.GetActions (context).Any (); if (!isValid) @@ -84,18 +97,29 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions return context.doc.Text; } - protected static void TestWrongContext (string input) where T : ICodeActionProvider, new () + protected void TestWrongContext (string input) where T : ICodeActionProvider, new () { TestWrongContext (new T(), input); } - protected static void TestWrongContext (ICodeActionProvider action, string input) + protected void TestWrongContext (ICodeActionProvider action, string input) { var context = TestRefactoringContext.Create (input); + context.FormattingOptions = formattingOptions; bool isValid = action.GetActions (context).Any (); if (!isValid) Console.WriteLine ("invalid node is:" + context.GetNode ()); Assert.IsTrue (!isValid, action.GetType () + " shouldn't be valid there."); } + + protected void TestActionDescriptions (ICodeActionProvider provider, string input, params string[] expected) + { + var ctx = TestRefactoringContext.Create(input); + ctx.FormattingOptions = formattingOptions; + var actions = provider.GetActions(ctx).ToList(); + Assert.AreEqual( + expected, + actions.Select(a => a.Description).ToArray()); + } } } diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/SortUsingsTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/SortUsingsTests.cs index 71af592bfd..af9b4c33ce 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/SortUsingsTests.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/SortUsingsTests.cs @@ -105,5 +105,22 @@ using System;", @"using System; using System.Linq;"); } + + + [Test] + public void TestPreservesPreprocessorDirectives() + { + Test(@"$using D; +using A; +#if true +using C; +using B; +#endif", @"using A; +using D; +#if true +using B; +using C; +#endif"); + } } } diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/TestRefactoringContext.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/TestRefactoringContext.cs index 915b3586b6..89e9ba55ea 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/TestRefactoringContext.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/TestRefactoringContext.cs @@ -55,6 +55,7 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions this.doc = document; this.location = location; this.UseExplicitTypes = UseExplict; + this.FormattingOptions = FormattingOptionsFactory.CreateMono (); UseExplict = false; Services.AddService (typeof(NamingConventionService), new TestNameService ()); } @@ -77,6 +78,8 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions get { return location; } } + public CSharpFormattingOptions FormattingOptions { get; set; } + public Script StartScript () { return new TestScript (this); @@ -85,7 +88,7 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions sealed class TestScript : DocumentScript { readonly TestRefactoringContext context; - public TestScript(TestRefactoringContext context) : base(context.doc, FormattingOptionsFactory.CreateMono (), new TextEditorOptions ()) + public TestScript(TestRefactoringContext context) : base(context.doc, context.FormattingOptions, new TextEditorOptions ()) { this.context = context; } diff --git a/ICSharpCode.NRefactory.Tests/FormattingTests/TestStatementIndentation.cs b/ICSharpCode.NRefactory.Tests/FormattingTests/TestStatementIndentation.cs index 71edbd340e..da18611dcd 100644 --- a/ICSharpCode.NRefactory.Tests/FormattingTests/TestStatementIndentation.cs +++ b/ICSharpCode.NRefactory.Tests/FormattingTests/TestStatementIndentation.cs @@ -2010,6 +2010,29 @@ if (b) { } }"); } + + [Test] + public void TestUsingInsideNamespace() + { + CSharpFormattingOptions policy = FormattingOptionsFactory.CreateMono(); + policy.UsingPlacement = UsingPlacement.InsideNamespace; + + Test(policy, @"namespace TestNamespace +{ +using System; + + class Test + { + } +}", @"namespace TestNamespace +{ + using System; + + class Test + { + } +}"); + } } } diff --git a/ICSharpCode.NRefactory.Tests/ICSharpCode.NRefactory.Tests.csproj b/ICSharpCode.NRefactory.Tests/ICSharpCode.NRefactory.Tests.csproj index 741fe1cf0c..7511debdec 100644 --- a/ICSharpCode.NRefactory.Tests/ICSharpCode.NRefactory.Tests.csproj +++ b/ICSharpCode.NRefactory.Tests/ICSharpCode.NRefactory.Tests.csproj @@ -75,6 +75,10 @@ + + + + @@ -388,6 +392,7 @@ +