From 219b884280245f21d0ab22e0abfa815f4eb926c3 Mon Sep 17 00:00:00 2001 From: CeddlyBurge Date: Thu, 29 Dec 2016 08:27:21 +0000 Subject: [PATCH 1/2] Add tests for ContractRequiresStringNotNullOrEmpty --- ...ntractRequiresStringNotNullOrEmptyTests.cs | 158 ++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 Tests/CSharp/CodeRefactorings/ContractRequiresStringNotNullOrEmptyTests.cs diff --git a/Tests/CSharp/CodeRefactorings/ContractRequiresStringNotNullOrEmptyTests.cs b/Tests/CSharp/CodeRefactorings/ContractRequiresStringNotNullOrEmptyTests.cs new file mode 100644 index 00000000..a5c80d3b --- /dev/null +++ b/Tests/CSharp/CodeRefactorings/ContractRequiresStringNotNullOrEmptyTests.cs @@ -0,0 +1,158 @@ +using System; +using NUnit.Framework; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis; +using System.Collections.Immutable; +using RefactoringEssentials.CSharp.CodeRefactorings; + +namespace RefactoringEssentials.Tests.CSharp.CodeRefactorings +{ + [TestFixture] + public class ContractRequiresStringNotNullOrEmptyTests : CSharpCodeRefactoringTestBase + { + [Test] + public void TestLambda() + { + Test(@"class Foo +{ + void Test () + { + var lambda = (string $s, int e) => { + }; + } +}", @"using System.Diagnostics.Contracts; + +class Foo +{ + void Test () + { + var lambda = (string s, int e) => { + Contract.Requires(string.IsNullOrEmpty(s) == false); + }; + } +}"); + } + + [Test] + public void TestAnonymousMethod() + { + Test(@"class Foo +{ + void Test () + { + var lambda = delegate(string $-[s]-, object e) { + }; + } +}", @"using System.Diagnostics.Contracts; + +class Foo +{ + void Test () + { + var lambda = delegate(string s, object e) { + Contract.Requires(string.IsNullOrEmpty(s) == false); + }; + } +}"); + } + + [Test] + public void TestContractAlreadyPresent() + { + TestWrongContext(@"class Foo +{ + void Test () + { + var lambda = (string $s, int e) => { + Contract.Requires(string.IsNullOrEmpty(s) == false); + }; + } +}"); + } + + [Test] + public void TestDifferentContractAlreadyPresent() + { + Test(@"class Foo +{ + void Test () + { + var lambda = (string $s, int e) => { + Contract.Requires(string.IsNullOrEmpty(notS) == false); + }; + } +}", @"using System.Diagnostics.Contracts; + +class Foo +{ + void Test () + { + var lambda = (string s, int e) => { + Contract.Requires(string.IsNullOrEmpty(s) == false); + Contract.Requires(string.IsNullOrEmpty(notS) == false); + }; + } +}"); + } + + [Test] + public void TestUsingStatementAlreadyPresent() + { + Test(@"using System.Diagnostics.Contracts; +class Foo +{ + void Test () + { + var lambda = (string $s, int e) => { + }; + } +}", @"using System.Diagnostics.Contracts; +class Foo +{ + void Test () + { + var lambda = (string s, int e) => { + Contract.Requires(string.IsNullOrEmpty(s) == false); + }; + } +}"); + } + + [Test] + public void TestPopupOnlyOnName() + { + TestWrongContext(@"class Foo +{ + void Test ($string param) + { + } +}"); + } + + [Test] + public void Test_OldCSharp() + { + var parseOptions = new CSharpParseOptions( + LanguageVersion.CSharp5, + DocumentationMode.Diagnose | DocumentationMode.Parse, + SourceCodeKind.Regular, + ImmutableArray.Create("DEBUG", "TEST") + ); + + Test(@"class Foo +{ + void Test (string $test) + { + } +}", @"using System.Diagnostics.Contracts; + +class Foo +{ + void Test (string test) + { + Contract.Requires(string.IsNullOrEmpty(test) == false); + } +}", parseOptions: parseOptions); + } + } +} From 29bb3d79415f356c8f2e4031d6f42845a2e5a0ff Mon Sep 17 00:00:00 2001 From: CeddlyBurge Date: Fri, 30 Dec 2016 18:42:20 +0000 Subject: [PATCH 2/2] Add implementation for ContractRequiresStringNotNullOrEmpty, fixes #209 --- .../RefactoringEssentials.csproj | 3 + ...EnsuresNotNullReturnRefactoringProvider.cs | 2 +- ...ngNotNullOrEmptyCodeRefactoringProvider.cs | 93 +++++++++++++++++++ .../CodeAnalyzers.CSharp.html | 3 +- .../CodeRefactorings.CSharp.html | 5 +- .../RefactoringEssentials.csproj | 1 + Tests.2017/Tests.csproj | 5 +- ...ntractRequiresStringNotNullOrEmptyTests.cs | 30 +++++- Tests/Tests.csproj | 1 + 9 files changed, 138 insertions(+), 5 deletions(-) create mode 100644 RefactoringEssentials/CSharp/CodeRefactorings/Synced/ContractRequiresStringNotNullOrEmptyCodeRefactoringProvider.cs diff --git a/RefactoringEssentials.2017/RefactoringEssentials.csproj b/RefactoringEssentials.2017/RefactoringEssentials.csproj index a56c0ba2..020c1fbb 100644 --- a/RefactoringEssentials.2017/RefactoringEssentials.csproj +++ b/RefactoringEssentials.2017/RefactoringEssentials.csproj @@ -169,6 +169,9 @@ CSharp\CodeRefactorings\Synced\ContractEnsuresNotNullReturnRefactoringProvider.cs + + CSharp\CodeRefactorings\Synced\ContractRequiresStringNotNullOrEmptyCodeRefactoringProvider.cs + CSharp\CodeRefactorings\Synced\ContractRequiresNotNullCodeRefactoringProvider.cs diff --git a/RefactoringEssentials/CSharp/CodeRefactorings/Synced/ContractEnsuresNotNullReturnRefactoringProvider.cs b/RefactoringEssentials/CSharp/CodeRefactorings/Synced/ContractEnsuresNotNullReturnRefactoringProvider.cs index e5e251c5..58318ae7 100644 --- a/RefactoringEssentials/CSharp/CodeRefactorings/Synced/ContractEnsuresNotNullReturnRefactoringProvider.cs +++ b/RefactoringEssentials/CSharp/CodeRefactorings/Synced/ContractEnsuresNotNullReturnRefactoringProvider.cs @@ -51,7 +51,7 @@ protected IEnumerable GetActions(Document document, SyntaxNode root, protected IEnumerable GetActions(Document document, SyntaxNode root, AccessorDeclarationSyntax node) { - var propertyOrIndexerDeclaration = node.Ancestors().Where(n => n.GetType().Equals(typeof(PropertyDeclarationSyntax)) || n.GetType().Equals(typeof(IndexerDeclarationSyntax))).FirstOrDefault(); + var propertyOrIndexerDeclaration = node.Ancestors().FirstOrDefault(n => n.GetType().Equals(typeof(PropertyDeclarationSyntax)) || n.GetType().Equals(typeof(IndexerDeclarationSyntax))); var nullableType = propertyOrIndexerDeclaration.ChildNodes().OfType().FirstOrDefault(); diff --git a/RefactoringEssentials/CSharp/CodeRefactorings/Synced/ContractRequiresStringNotNullOrEmptyCodeRefactoringProvider.cs b/RefactoringEssentials/CSharp/CodeRefactorings/Synced/ContractRequiresStringNotNullOrEmptyCodeRefactoringProvider.cs new file mode 100644 index 00000000..68209283 --- /dev/null +++ b/RefactoringEssentials/CSharp/CodeRefactorings/Synced/ContractRequiresStringNotNullOrEmptyCodeRefactoringProvider.cs @@ -0,0 +1,93 @@ +using System.Linq; +using System.Threading; +using System.Collections.Generic; +using Microsoft.CodeAnalysis.CodeRefactorings; +using Microsoft.CodeAnalysis; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.Text; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.Simplification; +using Microsoft.CodeAnalysis.Formatting; + +namespace RefactoringEssentials.CSharp.CodeRefactorings +{ + [ExportCodeRefactoringProvider(LanguageNames.CSharp, Name = "Add a Contract to specify the string parameter must not be null or empty")] + /// + /// Creates a 'Contract.Requires(param != null);' contract for a parameter. + /// + public class ContractRequiresStringNotNullOrEmptyCodeRefactoringProvider : CodeContractsCodeRefactoringProvider + { + #region ICodeActionProvider implementation + public override async Task ComputeRefactoringsAsync(CodeRefactoringContext context) + { + var codeContractsContext = await CodeContractsContext(context); + if (codeContractsContext == null) + return; + + var foundNode = (ParameterSyntax)codeContractsContext.Node.AncestorsAndSelf().FirstOrDefault(n => n is ParameterSyntax); + if (foundNode == null) + return; + + foreach (var action in GetActions(codeContractsContext.Document, codeContractsContext.SemanticModel, codeContractsContext.Root, codeContractsContext.TextSpan, foundNode)) + context.RegisterRefactoring(action); + } + #endregion + + protected IEnumerable GetActions(Document document, SemanticModel semanticModel, SyntaxNode root, TextSpan span, ParameterSyntax node) + { + if (!node.Identifier.Span.Contains(span)) + yield break; + + var parameter = node; + var bodyStatement = parameter.Parent.Parent.ChildNodes().OfType().FirstOrDefault(); + if (bodyStatement == null) + yield break; + + var parameterSymbol = semanticModel.GetDeclaredSymbol(node); + var type = parameterSymbol.Type; + if (type.SpecialType != SpecialType.System_String || HasReturnContract(bodyStatement, parameterSymbol.Name)) + yield break; + + yield return CreateAction( + node.Identifier.Span + , t2 => { + var newBody = bodyStatement.WithStatements(SyntaxFactory.List(new[] { CreateContractRequiresCall(node.Identifier.ToString()) }.Concat(bodyStatement.Statements))); + + var newRoot = (CompilationUnitSyntax)root.ReplaceNode((SyntaxNode)bodyStatement, newBody); + + if (UsingStatementNotPresent(newRoot)) newRoot = AddUsingStatement(node, newRoot); + + return Task.FromResult(document.WithSyntaxRoot(newRoot)); + } + , "Add contract requiring parameter must not be null or empty" + ); + } + + static StatementSyntax CreateContractRequiresCall(string parameterName) + { + return SyntaxFactory.ParseStatement($"Contract.Requires(string.IsNullOrEmpty({parameterName}) == false);\r\n").WithAdditionalAnnotations(Formatter.Annotation, Simplifier.Annotation); + } + + static bool HasReturnContract(BlockSyntax bodyStatement, string parameterName) + { + var workspace = new AdhocWorkspace(); + + foreach (var expression in bodyStatement.DescendantNodes().OfType()) + { + var formatted = Formatter.Format(expression, workspace).ToString(); + + if (formatted == $"Contract.Requires(string.IsNullOrEmpty({parameterName}) == false);") + return true; + + if (formatted == $"Contract.Requires(false == string.IsNullOrEmpty({parameterName}));") + return true; + + if (formatted == $"Contract.Requires(!string.IsNullOrEmpty({parameterName}));") + return true; + } + return false; + } + } +} diff --git a/RefactoringEssentials/CodeAnalyzers.CSharp.html b/RefactoringEssentials/CodeAnalyzers.CSharp.html index 2cba20b8..1fdf8bef 100644 --- a/RefactoringEssentials/CodeAnalyzers.CSharp.html +++ b/RefactoringEssentials/CodeAnalyzers.CSharp.html @@ -15,7 +15,7 @@ -->

Supported Code Analyzers

-

119 code analyzers for C#

+

120 code analyzers for C#

  • Suggests using the class declaring a static function when calling it (AccessToStaticMemberViaDerivedTypeAnalyzer)
  • When initializing explicitly typed local variable or array type, array creation expression can be replaced with array initializer. (ArrayCreationCanBeReplacedWithArrayInitializerAnalyzer)
    • @@ -131,6 +131,7 @@

    Supported Code Analyzers

  • Warns when a culture-aware 'IndexOf' call is used by default. (StringIndexOfIsCultureSpecificAnalyzer)
  • Warns when a culture-aware 'LastIndexOf' call is used by default. (StringLastIndexOfIsCultureSpecificAnalyzer)
  • Warns when a culture-aware 'StartsWith' call is used by default. (StringStartsWithIsCultureSpecificAnalyzer)
    • +
  • Use 'var' keyword when possible (SuggestUseVarKeywordEvidentAnalyzer)
  • [ThreadStatic] doesn't work with instance fields (ThreadStaticAtInstanceFieldAnalyzer)
  • Parameter is never used (UnusedParameterAnalyzer)
  • Type parameter is never used (UnusedTypeParameterAnalyzer)
    • diff --git a/RefactoringEssentials/CodeRefactorings.CSharp.html b/RefactoringEssentials/CodeRefactorings.CSharp.html index 7480f7ee..085072c3 100644 --- a/RefactoringEssentials/CodeRefactorings.CSharp.html +++ b/RefactoringEssentials/CodeRefactorings.CSharp.html @@ -15,7 +15,7 @@ -->

    Supported Refactorings

    -

    101 code refactorings for C#

    +

    104 code refactorings for C#

    • Adds another accessor (AddAnotherAccessorCodeRefactoringProvider)
    • Add braces (AddBracesCodeRefactoringProvider)
      • @@ -34,6 +34,7 @@

      Supported Refactorings

    • Compute constant value (ComputeConstantValueCodeRefactoringProvider)
    • Add a Contract to specify the return value must not be null (ContractEnsuresNotNullReturnCodeRefactoringProvider)
    • Add a Contract to specify the parameter must not be null (ContractRequiresNotNullCodeRefactoringProvider)
      • +
    • Add a Contract to specify the string parameter must not be null or empty (ContractRequiresStringNotNullOrEmptyCodeRefactoringProvider)
    • Convert anonymous method to lambda expression (ConvertAnonymousMethodToLambdaCodeRefactoringProvider)
    • Convert auto-property to computed propertyy (ConvertAutoPropertyToPropertyCodeRefactoringProvider)
    • Replace bitwise flag comparison with call to 'Enum.HasFlag' (ConvertBitwiseFlagComparisonToHasFlagsCodeRefactoringProvider)
      • @@ -102,10 +103,12 @@

      Supported Refactorings

    • Replace assignment with postfix expression (ReplaceAssignmentWithPostfixExpressionCodeRefactoringProvider)
    • Replace auto-property with property that uses a backing field (ReplaceAutoPropertyWithPropertyAndBackingFieldCodeRefactoringProvider)
    • Convert cast to 'as'. (ReplaceDirectCastWithSafeCastCodeRefactoringProvider)
      • +
    • Replace type with 'var' (ReplaceExplicitTypeWithVarCodeRefactoringProvider)
    • Replace operator assignment with assignment (ReplaceOperatorAssignmentWithAssignmentCodeRefactoringProvider)
    • Replace postfix expression with assignment (ReplacePostfixExpressionWithAssignmentCodeRefactoringProvider)
    • Replace property that uses a backing field with auto-property (ReplacePropertyWithBackingFieldWithAutoPropertyCodeRefactoringProvider)
    • Convert 'as' to cast. (ReplaceSafeCastWithDirectCastCodeRefactoringProvider)
      • +
    • Replaces 'var' with explicit type specification (ReplaceVarWithExplicitTypeCodeRefactoringProvider)
    • Replace assignment with operator assignment (ReplaceWithOperatorAssignmentCodeRefactoringProvider)
    • Reverse the direction of a for (ReverseDirectionForForLoopCodeRefactoringProvider)
    • Split declaration list (SplitDeclarationListCodeRefactoringProvider)
      • diff --git a/RefactoringEssentials/RefactoringEssentials.csproj b/RefactoringEssentials/RefactoringEssentials.csproj index 2203b1c4..919a3118 100644 --- a/RefactoringEssentials/RefactoringEssentials.csproj +++ b/RefactoringEssentials/RefactoringEssentials.csproj @@ -58,6 +58,7 @@ + diff --git a/Tests.2017/Tests.csproj b/Tests.2017/Tests.csproj index 4d3368af..7209b6c2 100644 --- a/Tests.2017/Tests.csproj +++ b/Tests.2017/Tests.csproj @@ -287,7 +287,10 @@ CSharp\CodeRefactorings\ContractEnsuresNotNullReturnTests.cs - + + CSharp\CodeRefactorings\ContractRequiresNotNullTests.cs + + CSharp\CodeRefactorings\ContractRequiresNotNullTests.cs diff --git a/Tests/CSharp/CodeRefactorings/ContractRequiresStringNotNullOrEmptyTests.cs b/Tests/CSharp/CodeRefactorings/ContractRequiresStringNotNullOrEmptyTests.cs index a5c80d3b..79657762 100644 --- a/Tests/CSharp/CodeRefactorings/ContractRequiresStringNotNullOrEmptyTests.cs +++ b/Tests/CSharp/CodeRefactorings/ContractRequiresStringNotNullOrEmptyTests.cs @@ -57,7 +57,7 @@ void Test () } [Test] - public void TestContractAlreadyPresent() + public void TestContractAlreadyPresentEqualsFalseFormat() { TestWrongContext(@"class Foo { @@ -70,6 +70,34 @@ void Test () }"); } + [Test] + public void TestContractAlreadyPresentFalseEqualsFormat() + { + TestWrongContext(@"class Foo +{ + void Test () + { + var lambda = (string $s, int e) => { + Contract.Requires(false==string.IsNullOrEmpty(s)); + }; + } +}"); + } + + [Test] + public void TestContractAlreadyPresentNegateFormat() + { + TestWrongContext(@"class Foo +{ + void Test () + { + var lambda = (string $s, int e) => { + Contract.Requires(!string.IsNullOrEmpty(s)); + }; + } +}"); + } + [Test] public void TestDifferentContractAlreadyPresent() { diff --git a/Tests/Tests.csproj b/Tests/Tests.csproj index 64c27dcb..56bb8497 100644 --- a/Tests/Tests.csproj +++ b/Tests/Tests.csproj @@ -135,6 +135,7 @@ +