Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add AG0044 rule - Add Analyzers for Force option methods #199

Merged
merged 2 commits into from
Nov 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 106 additions & 0 deletions doc/AG0044.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
# AG0044: Force option should not be used in Locator methods

## Problem Description
Using `Force = true` in Playwright's Locator methods bypasses important built-in validation checks. This can lead to brittle tests, inconsistent behavior, and difficulty in identifying real UI/UX issues.

## Rule Details
This rule raises an issue when `Force = true` is set in options objects passed to ILocator action methods.

### Noncompliant Code Examples

#### Using Force in Click Actions
```csharp
public async Task ClickButtonAsync(ILocator button)
{
// Noncompliant: Forces click without proper element state validation
await button.ClickAsync(new() { Force = true });
}
```

#### Using Force in Multiple Actions
```csharp
public async Task FillFormAsync(ILocator form)
{
var textbox = form.Locator(".textbox");
var checkbox = form.Locator(".checkbox");
var dropdown = form.Locator(".select");

// Noncompliant: Multiple forced interactions
await textbox.FillAsync("text", new() { Force = true });
await checkbox.CheckAsync(new() { Force = true });
await dropdown.SelectOptionAsync("option", new() { Force = true });
}
```

#### Pre-defined Options Object
```csharp
public async Task InteractWithElementAsync(ILocator element)
{
// Noncompliant: Force option in pre-defined options
var options = new LocatorClickOptions { Force = true };
await element.ClickAsync(options);
}
```

### Compliant Solutions

#### Using Default Behavior
```csharp
public async Task ClickButtonAsync(ILocator button)
{
// Compliant: Uses Playwright's built-in waiting and validation
await button.ClickAsync();
}
```

#### Using Proper Wait Strategies
```csharp
public async Task FillFormAsync(ILocator form)
{
var textbox = form.Locator(".textbox");
var checkbox = form.Locator(".checkbox");
var dropdown = form.Locator(".select");

// Compliant: Uses appropriate waiting and state checks
await textbox.WaitForAsync();
await textbox.FillAsync("text");

await checkbox.WaitForAsync(new() { State = WaitForSelectorState.Attached });
await checkbox.CheckAsync();

await dropdown.WaitForAsync(new() { State = WaitForSelectorState.Visible });
await dropdown.SelectOptionAsync("option");
}
```

#### Using Custom Timeout When Needed
```csharp
public async Task InteractWithSlowElementAsync(ILocator element)
{
// Compliant: Adjusts timeout instead of forcing interaction
await element.ClickAsync(new() { Timeout = 10000 });
}
```

## Why is this an Issue?
1. **Bypasses Important Checks**: Force option bypasses Playwright's built-in validations for:
- Element visibility
- Element being attached to DOM
- Element being enabled
- Element being stable (not moving)

2. **Masks Real Issues**: Using Force can hide actual problems in the application:
- Elements that are not properly visible to users
- Race conditions in UI rendering
- Timing issues in dynamic content loading
- Accessibility problems

3. **Unreliable Tests**: Tests using Force:
- May pass locally but fail in CI/CD
- Don't accurately reflect real user interactions
- Can become flaky or inconsistent
- Are harder to debug when they fail

## References
- [Playwright Actionability](https://playwright.dev/dotnet/docs/actionability)
- [Playwright Forcing Actions](https://playwright.dev/dotnet/docs/actionability#forcing-actions)
181 changes: 181 additions & 0 deletions src/Agoda.Analyzers.Test/AgodaCustom/AG0044UnitTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,181 @@
using System.Threading.Tasks;
using Agoda.Analyzers.AgodaCustom;
using Agoda.Analyzers.Test.Helpers;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.Playwright;
using NUnit.Framework;

namespace Agoda.Analyzers.Test.AgodaCustom;

class AG0044UnitTests : DiagnosticVerifier
{
protected override DiagnosticAnalyzer DiagnosticAnalyzer => new AG0044ForceOptionShouldNotBeUsed();

protected override string DiagnosticId => AG0044ForceOptionShouldNotBeUsed.DIAGNOSTIC_ID;

[Test]
public async Task AG0044_WhenUsingForceOptionInClickAsync_ShowWarning()
{
var code = new CodeDescriptor
{
References = new[] { typeof(ILocator).Assembly },
Code = @"
using System.Threading.Tasks;
using Microsoft.Playwright;

class TestClass
{
public async Task TestMethod(ILocator locator)
{
await locator.ClickAsync(new() { Force = true });
}
}"
};

await VerifyDiagnosticsAsync(code, new DiagnosticLocation(9, 58));
}

[Test]
public async Task AG0044_WhenUsingPreDefinedForceOption_ShowWarning()
{
var code = new CodeDescriptor
{
References = new[] { typeof(ILocator).Assembly },
Code = @"
using System.Threading.Tasks;
using Microsoft.Playwright;

class TestClass
{
public async Task TestMethod(ILocator locator)
{
var options = new LocatorClickOptions { Force = true };
await locator.ClickAsync(options);
}
}"
};

await VerifyDiagnosticsAsync(code, new DiagnosticLocation(9, 65));
}

[TestCase("HoverAsync", 58, TestName = "AG0044_WhenUsingForceInHoverAsync_ShowWarning")]
[TestCase("DblClickAsync", 61, TestName = "AG0044_WhenUsingForceInDblClickAsync_ShowWarning")]
[TestCase("TapAsync", 56, TestName = "AG0044_WhenUsingForceInTapAsync_ShowWarning")]
[TestCase("CheckAsync", 58, TestName = "AG0044_WhenUsingForceInCheckAsync_ShowWarning")]
[TestCase("UncheckAsync", 60, TestName = "AG0044_WhenUsingForceInUncheckAsync_ShowWarning")]
public async Task AG0044_WhenUsingForceOptionWithoutParams_ShowWarning(string methodName, int column)
{
var code = new CodeDescriptor
{
References = new[] { typeof(ILocator).Assembly },
Code = $@"
using System.Threading.Tasks;
using Microsoft.Playwright;

class TestClass
{{
public async Task TestMethod(ILocator locator)
{{
await locator.{methodName}(new() {{ Force = true }});
}}
}}"
};

await VerifyDiagnosticsAsync(code, new DiagnosticLocation(9, column));
}

[TestCase("FillAsync", "\"text\"", 65, TestName = "AG0044_WhenUsingForceInFillAsync_ShowWarning")]
[TestCase("SelectOptionAsync", "\"option\"", 75, TestName = "AG0044_WhenUsingForceInSelectOptionAsync_ShowWarning")]
public async Task AG0044_WhenUsingForceOptionWithParams_ShowWarning(string methodName, string parameter, int column)
{
var code = new CodeDescriptor
{
References = new[] { typeof(ILocator).Assembly },
Code = $@"
using System.Threading.Tasks;
using Microsoft.Playwright;

class TestClass
{{
public async Task TestMethod(ILocator locator)
{{
await locator.{methodName}({parameter}, new() {{ Force = true }});
}}
}}"
};

await VerifyDiagnosticsAsync(code, new DiagnosticLocation(9, column));
}

[Test]
public async Task AG0044_WhenNotUsingForceOption_NoWarning()
{
var code = new CodeDescriptor
{
References = new[] { typeof(ILocator).Assembly },
Code = @"
using System.Threading.Tasks;
using Microsoft.Playwright;

class TestClass
{
public async Task TestMethod(ILocator locator)
{
await locator.ClickAsync();
await locator.ClickAsync(new() { Timeout = 5000 });
var options = new LocatorClickOptions { Timeout = 5000 };
await locator.ClickAsync(options);
}
}"
};

await VerifyDiagnosticsAsync(code, EmptyDiagnosticResults);
}

[Test]
public async Task AG0044_WhenUsingCustomType_NoWarning()
{
var code = new CodeDescriptor
{
Code = @"
using System.Threading.Tasks;

class CustomLocator
{
public async Task ClickAsync(object options) { }
}

class TestClass
{
public async Task TestMethod()
{
var locator = new CustomLocator();
await locator.ClickAsync(new { Force = true });
}
}"
};

await VerifyDiagnosticsAsync(code, EmptyDiagnosticResults);
}

[Test]
public async Task AG0044_WhenSymbolIsNull_NoWarning()
{
var code = new CodeDescriptor
{
Code = @"
using System.Threading.Tasks;

class TestClass
{
public async Task TestMethod()
{
dynamic locator = null;
await locator.ClickAsync(new { Force = true });
}
}"
};

await VerifyDiagnosticsAsync(code, EmptyDiagnosticResults);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
using System.Collections.Immutable;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;

namespace Agoda.Analyzers.AgodaCustom
{
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class AG0044ForceOptionShouldNotBeUsed : DiagnosticAnalyzer
{
public const string DIAGNOSTIC_ID = "AG0044";
private const string Category = "Usage";

private static readonly LocalizableString Title = new LocalizableResourceString(
nameof(CustomRulesResources.AG0044Title),
CustomRulesResources.ResourceManager,
typeof(CustomRulesResources));

private static readonly LocalizableString MessageFormat = new LocalizableResourceString(
nameof(CustomRulesResources.AG0044MessageFormat),
CustomRulesResources.ResourceManager,
typeof(CustomRulesResources));

private static readonly LocalizableString Description = new LocalizableResourceString(
nameof(CustomRulesResources.AG0044Description),
CustomRulesResources.ResourceManager,
typeof(CustomRulesResources));

private static readonly DiagnosticDescriptor Rule = new DiagnosticDescriptor(
DIAGNOSTIC_ID,
Title,
MessageFormat,
Category,
DiagnosticSeverity.Warning,
isEnabledByDefault: true,
description: Description,
"https://github.com/agoda-com/AgodaAnalyzers/blob/master/doc/AG0044.md",
WellKnownDiagnosticTags.EditAndContinue);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);

public override void Initialize(AnalysisContext context)
{
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.EnableConcurrentExecution();
context.RegisterSyntaxNodeAction(AnalyzeNode, SyntaxKind.ObjectInitializerExpression);
}

private void AnalyzeNode(SyntaxNodeAnalysisContext context)
{
var initializerExpression = (InitializerExpressionSyntax)context.Node;
if (initializerExpression.Parent == null)
return;

Check warning on line 54 in src/Agoda.Analyzers/AgodaCustom/AG0044ForceOptionShouldNotBeUsed.cs

View check run for this annotation

Codecov / codecov/patch

src/Agoda.Analyzers/AgodaCustom/AG0044ForceOptionShouldNotBeUsed.cs#L54

Added line #L54 was not covered by tests

// Check if this is initializing an options object for ILocator methods
var typeInfo = context.SemanticModel.GetTypeInfo(initializerExpression.Parent);
if (typeInfo.Type == null) return;

var typeName = typeInfo.Type.Name;
if (!IsLocatorOptionsType(typeName)) return;

// Look for Force = true in the initializer
foreach (var expression in initializerExpression.Expressions)
{
if (!(expression is AssignmentExpressionSyntax assignment) ||
!(assignment.Left is IdentifierNameSyntax identifier) ||
identifier.Identifier.ValueText != "Force") continue;

if (!(assignment.Right is LiteralExpressionSyntax literal) ||
literal.Token.ValueText != "true") continue;

var diagnostic = Diagnostic.Create(Rule, expression.GetLocation());
context.ReportDiagnostic(diagnostic);
}
}

private static bool IsLocatorOptionsType(string typeName)
{
return typeName.EndsWith("Options") && (
typeName.Contains("LocatorClick") ||
typeName.Contains("LocatorFill") ||
typeName.Contains("LocatorHover") ||
typeName.Contains("LocatorDblClick") ||
typeName.Contains("LocatorTap") ||
typeName.Contains("LocatorCheck") ||
typeName.Contains("LocatorUncheck") ||
typeName.Contains("LocatorSelectOption"));
}
}
}
Loading
Loading