Merge pull request #198 from agoda-com/AG0042_update

Update the AG0042 rule to be more generic

doc/AG0042.md

@@ -1,61 +1,75 @@
-# AG0042: QuerySelector should not be used with Playwright
+# AG0042: ElementHandle methods should not be used with Playwright
 
 ## Problem Description
 
-Using `QuerySelectorAsync()` in Playwright tests can lead to brittle and unreliable tests. This method uses CSS selectors which can be fragile and may break when the UI structure changes. Instead, more reliable locator strategies like data-testid or role-based selectors should be used.
+Using methods that return ElementHandle (such as `QuerySelectorAsync()`, `QuerySelectorAllAsync()`, `WaitForSelectorAsync()`, etc.) in Playwright tests can lead to brittle and unreliable tests. These methods often rely on CSS selectors which can be fragile and may break when the UI structure changes. Instead, more reliable locator strategies like data-testid or role-based selectors should be used.
 
 ## Rule Details
 
-This rule raises an issue when `QuerySelectorAsync()` is called on Playwright `IPage` or `Page` objects.
+This rule raises an issue when any method returning an `IElementHandle`, `ElementHandle`, or their nullable variants is called on Playwright `IPage`, `Page`, or `IElementHandle` objects.
 
-### Noncompliant Code Example
+### Noncompliant Code Examples
 
 ```csharp
-public async Task ClickLoginButton(IPage page)
+public async Task InteractWithElements(IPage page)
 {
-    // Noncompliant: Using QuerySelectorAsync with CSS selector
+    // Noncompliant: Using methods that return ElementHandle
     var loginButton = await page.QuerySelectorAsync(".login-button");
-    await loginButton.ClickAsync();
+    var menuItems = await page.QuerySelectorAllAsync(".menu-item");
+    var dynamicElement = await page.WaitForSelectorAsync("#dynamic-content");
+
+    // Noncompliant: Chaining ElementHandle operations
+    var childElement = await loginButton.QuerySelectorAsync(".child");
 }
 ```
 
 ### Compliant Solution
 
 ```csharp
-public async Task ClickLoginButton(IPage page)
+public async Task InteractWithElements(IPage page)
 {
     // Compliant: Using Locator with data-testid
-    await page.Locator("[data-testid='login-button']").ClickAsync();
+    await page.GetByTestId("login-button").ClickAsync();
 
     // Compliant: Using role-based selector
     await page.GetByRole(AriaRole.Button, new() { Name = "Login" }).ClickAsync();
 
     // Compliant: Using text content
     await page.GetByText("Login").ClickAsync();
+
+    // Compliant: Working with multiple elements
+    var menuItems = page.Locator("[data-testid='menu-item']");
+    await menuItems.First.ClickAsync();
+
+    // Compliant: Waiting for elements
+    await page.Locator("#dynamic-content").WaitForAsync();
 }
 ```
 
 ## Why is this an Issue?
 
-1. **Fragile Selectors**: CSS selectors are tightly coupled to the DOM structure and styling classes, making tests brittle when:
-    - CSS classes are renamed or removed
-    - DOM hierarchy changes
-    - Styling frameworks are updated
+1. **ElementHandle Limitations**: ElementHandle references can become stale when the DOM changes, leading to flaky tests.
 
-2. **Maintainability**: CSS selectors can be complex and hard to maintain, especially when dealing with nested elements or specific combinations of classes.
+2. **Fragile Selectors**: CSS selectors are tightly coupled to the DOM structure and styling classes, making tests brittle when:
+   - CSS classes are renamed or removed
+   - DOM hierarchy changes
+   - Styling frameworks are updated
 
-3. **Best Practices**: Playwright provides better alternatives that are:
-    - More resilient to changes
-    - More readable and maintainable
-    - Better aligned with testing best practices
+3. **Maintainability**: ElementHandle-based selectors can be complex and hard to maintain, especially when dealing with nested elements.
+
+4. **Best Practices**: Playwright provides better alternatives through Locators that are:
+   - More resilient to changes
+   - More readable and maintainable
+   - Better aligned with testing best practices
+   - Auto-waiting and retry-able
 
 ## Better Alternatives
 
-Playwright provides several better methods for selecting elements:
+Playwright provides several better methods for selecting and interacting with elements:
 
 1. **Data Test IDs**:
    ```csharp
-   await page.Locator("[data-testid='submit-button']").ClickAsync();
+   await page.GetByTestId("submit-button").ClickAsync();
    ```
 
 2. **Role-based Selectors**:
@@ -75,49 +89,6 @@ Playwright provides several better methods for selecting elements:
    await page.GetByPlaceholder("Enter email").FillAsync("[email protected]");
    ```
 
-## How to Fix It
-
-1. Replace `QuerySelectorAsync()` calls with more specific Playwright locators:
-
-   ```csharp
-   // Before
-   var element = await page.QuerySelectorAsync(".submit-btn");
-
-   // After
-   var element = page.GetByRole(AriaRole.Button, new() { Name = "Submit" });
-   ```
-
-2. Add data-testid attributes to your application's elements:
-   ```html
-   <button data-testid="submit-button">Submit</button>
-   ```
-
-   ```csharp
-   await page.Locator("[data-testid='submit-button']").ClickAsync();
-   ```
-
-3. Use semantic HTML with ARIA roles and labels:
-   ```html
-   <button role="button" aria-label="Submit form">Submit</button>
-   ```
-
-   ```csharp
-   await page.GetByRole(AriaRole.Button, new() { Name = "Submit form" }).ClickAsync();
-   ```
-
-## Exceptions
-
-This rule might be relaxed in the following scenarios:
-- Legacy test code that's pending migration
-- Complex third-party components where other selectors aren't available
-- Testing CSS-specific functionality
-
-## Benefits
-- More reliable tests
-- Better test maintenance
-- Clearer test intentions
-- Improved accessibility testing
-
 ## References
 - [ElementHandle is Discouraged by official Documents](https://playwright.dev/dotnet/docs/api/class-elementhandle)
 - [Playwright Locators Documentation](https://playwright.dev/docs/locators)

src/Agoda.Analyzers.Test/AgodaCustom/AG0042UnitTests.cs

@@ -9,9 +9,144 @@ namespace Agoda.Analyzers.Test.AgodaCustom;
 
 class AG0042UnitTests : DiagnosticVerifier
 {
-    protected override DiagnosticAnalyzer DiagnosticAnalyzer => new AG0042QuerySelectorShouldNotBeUsed();
+    protected override DiagnosticAnalyzer DiagnosticAnalyzer => new AG0042ElementHandlerShouldNotBeUsed();
 
-    protected override string DiagnosticId => AG0042QuerySelectorShouldNotBeUsed.DIAGNOSTIC_ID;
+    protected override string DiagnosticId => AG0042ElementHandlerShouldNotBeUsed.DIAGNOSTIC_ID;
+
+    [TestCase("QuerySelectorAsync")]
+    [TestCase("QuerySelectorAllAsync")]
+    [TestCase("WaitForSelectorAsync")]
+    public async Task AG0042_WhenUsingElementHandlerMethodWithPlaywrightPage_ShowWarning(string methodName)
+    {
+        var code = new CodeDescriptor
+        {
+            References = new[] { typeof(IPage).Assembly },
+            Code = $@"
+                    using System.Threading.Tasks;
+                    using Microsoft.Playwright;
+
+                    class TestClass
+                    {{
+                        public async Task TestMethod(IPage page)
+                        {{
+                            await page.{methodName}(""#element"");
+                        }}
+                    }}"
+        };
+
+        await VerifyDiagnosticsAsync(code, new DiagnosticLocation(9, 35));
+    }
+
+    [Test]
+    public async Task AG0042_WhenUsingLocatorMethod_NoWarning()
+    {
+        var code = new CodeDescriptor
+        {
+            References = new[] { typeof(IPage).Assembly },
+            Code = @"
+                    using System.Threading.Tasks;
+                    using Microsoft.Playwright;
+
+                    class TestClass
+                    {
+                        public void TestMethod(IPage page)
+                        {
+                            var element = page.Locator(""[data-testid='element']"");
+                            element.GetByRole(AriaRole.Button).ClickAsync();
+                        }
+                    }"
+        };
+
+        await VerifyDiagnosticsAsync(code, EmptyDiagnosticResults);
+    }
+
+    [Test]
+    public async Task AG0042_WhenUsingMethodReturningElementHandleArray_ShowWarning()
+    {
+        var code = new CodeDescriptor
+        {
+            References = new[] { typeof(IPage).Assembly },
+            Code = @"
+            using System.Threading.Tasks;
+            using Microsoft.Playwright;
+
+            class TestClass
+            {
+                public async Task TestMethod(IPage page)
+                {
+                    await page.QuerySelectorAllAsync(""#element"");
+                }
+            }"
+        };
+
+        await VerifyDiagnosticsAsync(code, new DiagnosticLocation(9, 27));
+    }
+
+    [Test]
+    public async Task AG0042_WhenUsingMethodReturningNonTaskType_NoWarning()
+    {
+        var code = new CodeDescriptor
+        {
+            References = new[] { typeof(IPage).Assembly },
+            Code = @"
+            using System.Threading.Tasks;
+            using Microsoft.Playwright;
+
+            class TestClass
+            {
+                public void TestMethod(IPage page)
+                {
+                    var result = page.ToString();
+                }
+            }"
+        };
+
+        await VerifyDiagnosticsAsync(code, EmptyDiagnosticResults);
+    }
+
+    [Test]
+    public async Task AG0042_WhenUsingMethodWithGenericTaskButNotElementHandle_NoWarning()
+    {
+        var code = new CodeDescriptor
+        {
+            References = new[] { typeof(IPage).Assembly },
+            Code = @"
+            using System.Threading.Tasks;
+            using Microsoft.Playwright;
+
+            class TestClass
+            {
+                public async Task TestMethod(IPage page)
+                {
+                    await page.EvaluateAsync<string>(""() => 'test'"");
+                }
+            }"
+        };
+
+        await VerifyDiagnosticsAsync(code, EmptyDiagnosticResults);
+    }
+
+    [Test]
+    public async Task AG0042_WhenMethodSymbolIsNull_NoWarning()
+    {
+        var code = new CodeDescriptor
+        {
+            Code = @"
+            using System.Threading.Tasks;
+
+            class TestClass
+            {
+                public async Task TestMethod()
+                {
+                    dynamic page = null;
+                    // This will make methodSymbol null since it's dynamic
+                    await page.NonExistentMethod(""#element"");
+                }
+            }"
+        };
+
+        await VerifyDiagnosticsAsync(code, EmptyDiagnosticResults);
+    }
 
     [Test]
     public async Task AG0042_WhenUsingQuerySelectorAsyncWithPlaywrightPage_ShowWarning()