-
Notifications
You must be signed in to change notification settings - Fork 12
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #199 from agoda-com/rparn/AG0044
Add AG0044 rule - Add Analyzers for Force option methods
- Loading branch information
Showing
6 changed files
with
406 additions
and
3 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
181
src/Agoda.Analyzers.Test/AgodaCustom/AG0044UnitTests.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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); | ||
} | ||
} |
91 changes: 91 additions & 0 deletions
91
src/Agoda.Analyzers/AgodaCustom/AG0044ForceOptionShouldNotBeUsed.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 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")); | ||
} | ||
} | ||
} |
Oops, something went wrong.