Skip to content

Commit

Permalink
Revert "WIP"
Browse files Browse the repository at this point in the history 
This reverts commit 84ea3f5.
  • Loading branch information
Mikkel Bundgaard committed Feb 18, 2024
1 parent 84ea3f5 commit 2bced5a
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 218 deletions.
95 changes: 0 additions & 95 deletions src/nunit.analyzers.tests/ConstraintsUsage/EqualConstraintUsageAnalyzerTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,6 @@ public class EqualConstraintUsageAnalyzerTests
ExpectedDiagnostic.Create(AnalyzerIdentifiers.EqualConstraintUsage,
string.Format(CultureInfo.InvariantCulture, EqualConstraintUsageConstants.Message, "Is.Not.EqualTo"));

private static readonly ExpectedDiagnostic isNullDiagnostic =
ExpectedDiagnostic.Create(AnalyzerIdentifiers.EqualConstraintUsage,
string.Format(CultureInfo.InvariantCulture, EqualConstraintUsageConstants.Message, "Is.Null"));
private static readonly ExpectedDiagnostic isNotNullDiagnostic =
ExpectedDiagnostic.Create(AnalyzerIdentifiers.EqualConstraintUsage,
string.Format(CultureInfo.InvariantCulture, EqualConstraintUsageConstants.Message, "Is.Not.Null"));

[Test]
public void AnalyzeWhenEqualsOperatorUsed()
{
Expand All @@ -38,28 +31,6 @@ public void AnalyzeWhenEqualsOperatorUsed()
RoslynAssert.Diagnostics(analyzer, isEqualToDiagnostic, testCode);
}

[Theory]
[TestCase("actual", "null")]
[TestCase("null", "actual")]
public void AnalyzeWhenEqualsOperatorUsedForNull(string leftOperand, string rightOperand)
{
var testCode = TestUtility.WrapInTestMethod(@$"
var actual = ""abc"";
Assert.That(↓{leftOperand} == {rightOperand});");

RoslynAssert.Diagnostics(analyzer, isNullDiagnostic, testCode);
}

[Test]
public void AnalyzeWhenPatternOperatorUsed()
{
var testCode = TestUtility.WrapInTestMethod(@"
var actual = ""abc"";
Assert.That(↓actual is null);");

RoslynAssert.Diagnostics(analyzer, isNullDiagnostic, testCode);
}

[Test]
public void AnalyzeWhenNotEqualsOperatorUsed()
{
Expand All @@ -70,28 +41,6 @@ public void AnalyzeWhenNotEqualsOperatorUsed()
RoslynAssert.Diagnostics(analyzer, isNotEqualToDiagnostic, testCode);
}

[Theory]
[TestCase("actual", "null")]
[TestCase("null", "actual")]
public void AnalyzeWhenNotEqualsOperatorUsedForNull(string leftOperand, string rightOperand)
{
var testCode = TestUtility.WrapInTestMethod(@$"
var actual = ""abc"";
Assert.That(↓{leftOperand} != {rightOperand});");

RoslynAssert.Diagnostics(analyzer, isNotNullDiagnostic, testCode);
}

[Test]
public void AnalyzeWhenNegatedPatternOperatorUsed()
{
var testCode = TestUtility.WrapInTestMethod(@"
var actual = ""abc"";
Assert.That(↓actual is not null);");

RoslynAssert.Diagnostics(analyzer, isNotNullDiagnostic, testCode);
}

[Test]
public void AnalyzeWhenEqualsInstanceMethodUsed()
{
Expand All @@ -102,16 +51,6 @@ public void AnalyzeWhenEqualsInstanceMethodUsed()
RoslynAssert.Diagnostics(analyzer, isEqualToDiagnostic, testCode);
}

[Test]
public void AnalyzeWhenEqualsInstanceMethodUsedWithNullParameter()
{
var testCode = TestUtility.WrapInTestMethod(@"
var actual = ""abc"";
Assert.That(↓actual.Equals(null));");

RoslynAssert.Diagnostics(analyzer, isNullDiagnostic, testCode);
}

[Test]
public void AnalyzeWhenNegatedEqualsInstanceMethodUsed()
{
Expand All @@ -122,16 +61,6 @@ public void AnalyzeWhenNegatedEqualsInstanceMethodUsed()
RoslynAssert.Diagnostics(analyzer, isNotEqualToDiagnostic, testCode);
}

[Test]
public void AnalyzeWhenNegatedEqualsInstanceMethodUsedWithNullParameter()
{
var testCode = TestUtility.WrapInTestMethod(@"
var actual = ""abc"";
Assert.That(↓!actual.Equals(null));");

RoslynAssert.Diagnostics(analyzer, isNotNullDiagnostic, testCode);
}

[Test]
public void AnalyzeWhenEqualsStaticMethodUsed()
{
Expand All @@ -142,18 +71,6 @@ public void AnalyzeWhenEqualsStaticMethodUsed()
RoslynAssert.Diagnostics(analyzer, isEqualToDiagnostic, testCode);
}

[Theory]
[TestCase("actual", "null")]
[TestCase("null", "actual")]
public void AnalyzeWhenEqualsStaticMethodUsedForNull(string leftOperand, string rightOperand)
{
var testCode = TestUtility.WrapInTestMethod(@$"
var actual = ""abc"";
Assert.That(↓Equals({leftOperand}, {rightOperand}));");

RoslynAssert.Diagnostics(analyzer, isNullDiagnostic, testCode);
}

[Test]
public void AnalyzeWhenNegatedEqualsStaticMethodUsed()
{
Expand All @@ -164,18 +81,6 @@ public void AnalyzeWhenNegatedEqualsStaticMethodUsed()
RoslynAssert.Diagnostics(analyzer, isNotEqualToDiagnostic, testCode);
}

[Theory]
[TestCase("actual", "null")]
[TestCase("null", "actual")]
public void AnalyzeWhenNegatedEqualsStaticMethodUsedForNull(string leftOperand, string rightOperand)
{
var testCode = TestUtility.WrapInTestMethod(@$"
var actual = ""abc"";
Assert.That(↓!Equals({leftOperand}, {rightOperand}));");

RoslynAssert.Diagnostics(analyzer, isNotNullDiagnostic, testCode);
}

[Test]
public void AnalyzeWhenEqualsMethodUsedWithIsTrue()
{
Expand Down
14 changes: 0 additions & 14 deletions src/nunit.analyzers.tests/ConstraintsUsage/EqualConstraintUsageCodeFixTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,20 +27,6 @@ public void FixesEqualsOperator()
RoslynAssert.CodeFix(analyzer, fix, equalConstraintDiagnostic, code, fixedCode);
}

[Test]
public void FixesEqualsOperatorWithNull()
{
var code = TestUtility.WrapInTestMethod(@"
var actual = ""abc"";
Assert.That(actual == null);");

var fixedCode = TestUtility.WrapInTestMethod(@"
var actual = ""abc"";
Assert.That(actual, Is.Null);");

RoslynAssert.CodeFix(analyzer, fix, equalConstraintDiagnostic, code, fixedCode);
}

[Test]
public void FixesNotEqualsOperator()
{
Expand Down
56 changes: 4 additions & 52 deletions src/nunit.analyzers/ConstraintUsage/BaseConditionConstraintAnalyzer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,45 +33,12 @@ protected static bool IsRefStruct(IOperation operation)
return operation.Type?.TypeKind == TypeKind.Struct && operation.Type.IsRefLikeType;
}

protected static bool IsBinaryOperationNotUsingRefStructOperands(
IOperation actual,
BinaryOperatorKind binaryOperator,
out bool atLeastOneOperandIsNull)
protected static bool IsBinaryOperationNotUsingRefStructOperands(IOperation actual, BinaryOperatorKind binaryOperator)
{
atLeastOneOperandIsNull = false;

var binaryOperation = actual as IBinaryOperation;

if (binaryOperation is null || binaryOperation.OperatorKind != binaryOperator)
{
return false;
}

var x = !IsRefStruct(binaryOperation.LeftOperand) &&
return actual is IBinaryOperation binaryOperation &&
binaryOperation.OperatorKind == binaryOperator &&
!IsRefStruct(binaryOperation.LeftOperand) &&
!IsRefStruct(binaryOperation.RightOperand);

if (!x)
{
return x;
}

atLeastOneOperandIsNull = IsOperationWithNullConstant(binaryOperation.LeftOperand)
|| IsOperationWithNullConstant(binaryOperation.RightOperand);

return x;
}

protected static bool IsPatternOperationWithConstantNull(IOperation actual)
{
return actual is IIsPatternOperation isPatternOperation &&
IsConstantPatternWithNull(isPatternOperation.Pattern);
}

protected static bool IsNegatedPatternOperationWithConstantNull(IOperation actual)
{
return actual is IIsPatternOperation isPatternOperation1 &&
isPatternOperation1.Pattern is INegatedPatternOperation negatedPattern &&
IsConstantPatternWithNull(negatedPattern.Pattern);
}

protected virtual (DiagnosticDescriptor? descriptor, string? suggestedConstraint, bool swapOperands) GetDiagnosticDataWithPossibleSwapOperands(
Expand Down Expand Up @@ -142,21 +109,6 @@ protected override void AnalyzeAssertInvocation(OperationAnalysisContext context
}
}

protected static bool IsOperationWithNullConstant(IOperation operation)
{
return operation is IConversionOperation conversionOperation &&
conversionOperation.Operand is ILiteralOperation operand &&
operand.ConstantValue.HasValue &&
operand.ConstantValue.Value is null;
}

private static bool IsConstantPatternWithNull(IPatternOperation operation)
{
return operation is IConstantPatternOperation constantPattern &&
constantPattern.Value.ConstantValue.HasValue &&
constantPattern.Value.ConstantValue.Value is null;
}

private static bool IsPrefixNotOperation(IOperation operation, [NotNullWhen(true)] out IOperation? operand)
{
if (operation is IUnaryOperation unaryOperation
Expand Down
61 changes: 9 additions & 52 deletions src/nunit.analyzers/ConstraintUsage/EqualConstraintUsageAnalyzer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ public class EqualConstraintUsageAnalyzer : BaseConditionConstraintAnalyzer
{
private const string IsEqualTo = $"{NameOfIs}.{NameOfIsEqualTo}";
private const string IsNotEqualTo = $"{NameOfIs}.{NameOfIsNot}.{NameOfIsEqualTo}";
private const string IsNull = $"{NameOfIs}.{NameOfIsNull}";
private const string IsNotNotNull = $"{NameOfIs}.{NameOfIsNot}.{NameOfIsNull}";

private static readonly DiagnosticDescriptor descriptor = DiagnosticDescriptorCreator.Create(
id: AnalyzerIdentifiers.EqualConstraintUsage,
Expand All @@ -29,101 +27,60 @@ protected override (DiagnosticDescriptor? descriptor, string? suggestedConstrain
OperationAnalysisContext context, IOperation actual, bool negated)
{
var shouldReport = false;
bool isNull;

// 'actual == expected', 'Equals(actual, expected)' or 'actual.Equals(expected)'
if (IsBinaryOperationNotUsingRefStructOperands(actual, BinaryOperatorKind.Equals, out isNull)
|| IsStaticObjectEquals(actual, out isNull)
|| IsInstanceObjectEquals(actual, out isNull))
if (IsBinaryOperationNotUsingRefStructOperands(actual, BinaryOperatorKind.Equals)
|| IsStaticObjectEquals(actual)
|| IsInstanceObjectEquals(actual))
{
shouldReport = true;
}

// 'actual is null'
else if (IsPatternOperationWithConstantNull(actual))
{
shouldReport = true;
isNull = true;
}

// 'actual != expected'
else if (IsBinaryOperationNotUsingRefStructOperands(actual, BinaryOperatorKind.NotEquals, out isNull))
else if (IsBinaryOperationNotUsingRefStructOperands(actual, BinaryOperatorKind.NotEquals))
{
shouldReport = true;
negated = !negated;
}

// 'actual is not null'
else if (IsNegatedPatternOperationWithConstantNull(actual))
{
shouldReport = true;
isNull = true;
negated = !negated;
}

if (shouldReport)
{
var suggestedConstraint = isNull
? negated ? IsNotNotNull : IsNull
: negated ? IsNotEqualTo : IsEqualTo;
var suggestedConstraint = negated ? IsNotEqualTo : IsEqualTo;
return (descriptor, suggestedConstraint);
}

return (null, null);
}

private static bool IsStaticObjectEquals(IOperation operation, out bool atLeastOneOperandIsNull)
private static bool IsStaticObjectEquals(IOperation operation)
{
atLeastOneOperandIsNull = false;

if (operation is not IInvocationOperation invocation)
return false;

var methodSymbol = invocation.TargetMethod;

var result = methodSymbol is not null
return methodSymbol is not null
&& methodSymbol.IsStatic
&& methodSymbol.Parameters.Length == 2
&& methodSymbol.Name == nameof(object.Equals)
&& methodSymbol.ContainingType.SpecialType == SpecialType.System_Object;

if (!result)
{
return false;
}

atLeastOneOperandIsNull =
IsOperationWithNullConstant(invocation.Arguments[0].Value)
|| IsOperationWithNullConstant(invocation.Arguments[1].Value);

return true;
}

private static bool IsInstanceObjectEquals(IOperation operation, out bool isArgumentNull)
private static bool IsInstanceObjectEquals(IOperation operation)
{
isArgumentNull = false;

if (operation is not IInvocationOperation invocation)
return false;

var methodSymbol = invocation.TargetMethod;

var result = methodSymbol is not null
return methodSymbol is not null
&& !methodSymbol.IsStatic
&& methodSymbol.Parameters.Length == 1
&& methodSymbol.Name == nameof(object.Equals)
&& invocation.Arguments.Length == 1
&& !IsRefStruct(invocation.Arguments[0])
&& invocation.Instance is not null
&& !IsRefStruct(invocation.Instance);

if (!result)
{
return false;
}

isArgumentNull = IsOperationWithNullConstant(invocation.Arguments[0].Value);
return true;
}
}
}
6 changes: 1 addition & 5 deletions src/nunit.analyzers/ConstraintUsage/EqualConstraintUsageCodeFix.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,8 @@ protected override (ExpressionSyntax? actual, ExpressionSyntax? constraintExpres
private static (ExpressionSyntax? actual, ExpressionSyntax? expected) GetActualExpected(SyntaxNode conditionNode)
{
if (conditionNode is BinaryExpressionSyntax binaryExpression &&
(binaryExpression.IsKind(SyntaxKind.EqualsExpression) ||
binaryExpression.IsKind(SyntaxKind.NotEqualsExpression) ||
binaryExpression.IsKind(SyntaxKind.IsExpression) ||
binaryExpression.IsKind(SyntaxKind.IsPatternExpression)))
(binaryExpression.IsKind(SyntaxKind.EqualsExpression) || binaryExpression.IsKind(SyntaxKind.NotEqualsExpression)))
{
// TODO we should handle is different
return (binaryExpression.Left, binaryExpression.Right);
}
else
Expand Down

0 comments on commit 2bced5a

Please sign in to comment.