Skip to content

Commit

Permalink
Add analyzer for QuerySelector (#191)
Browse files Browse the repository at this point in the history 
* feat: Add analyzer for QuerySelector

* docs: add MD doc for rule

* feat: fix code and add more tests

* feat: Add analyzer for QuerySelector

* docs: add MD doc for rule

* feat: fix code and add more tests

* docs: move the MD to correct folder

* remove docs folder and files

* docs: let Claude generate docs

* Update CustomRulesResources.resx

* fix: AnalyzerRelease warning

---------

Co-authored-by: Joel Dickson <[email protected]>
  • Loading branch information
@rannes @joeldickson
rannes and joeldickson authored Oct 17, 2024
1 parent 57aeae5 commit 7e9c705
Show file tree
Hide file tree
Showing 6 changed files with 476 additions and 0 deletions.
123 changes: 123 additions & 0 deletions doc/AG0042.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
# AG0042: QuerySelector 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.

## Rule Details

This rule raises an issue when `QuerySelectorAsync()` is called on Playwright `IPage` or `Page` objects.

### Noncompliant Code Example

```csharp
public async Task ClickLoginButton(IPage page)
{
// Noncompliant: Using QuerySelectorAsync with CSS selector
var loginButton = await page.QuerySelectorAsync(".login-button");
await loginButton.ClickAsync();
}
```

### Compliant Solution

```csharp
public async Task ClickLoginButton(IPage page)
{
// Compliant: Using Locator with data-testid
await page.Locator("[data-testid='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();
}
```

## 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

2. **Maintainability**: CSS selectors can be complex and hard to maintain, especially when dealing with nested elements or specific combinations of classes.

3. **Best Practices**: Playwright provides better alternatives that are:
- More resilient to changes
- More readable and maintainable
- Better aligned with testing best practices

## Better Alternatives

Playwright provides several better methods for selecting elements:

1. **Data Test IDs**:
```csharp
await page.Locator("[data-testid='submit-button']").ClickAsync();
```

2. **Role-based Selectors**:
```csharp
await page.GetByRole(AriaRole.Button).ClickAsync();
await page.GetByRole(AriaRole.Textbox, new() { Name = "Username" }).FillAsync("user");
```

3. **Text Content**:
```csharp
await page.GetByText("Sign up").ClickAsync();
await page.GetByLabel("Password").FillAsync("secret");
```

4. **Placeholder Text**:
```csharp
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)
244 changes: 244 additions & 0 deletions src/Agoda.Analyzers.Test/AgodaCustom/AG0042UnitTests.cs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,244 @@
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 AG0042UnitTests : DiagnosticVerifier
{
protected override DiagnosticAnalyzer DiagnosticAnalyzer => new AG0042QuerySelectorShouldNotBeUsed();

protected override string DiagnosticId => AG0042QuerySelectorShouldNotBeUsed.DIAGNOSTIC_ID;

[Test]
public async Task AG0042_WhenUsingQuerySelectorAsyncWithPlaywrightPage_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.QuerySelectorAsync(""#element"");
}
}"
};

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

[Test]
public async Task AG0042_WhenUsingQuerySelectorAsyncWithIPageInstanceVariable_ShowWarning()
{
var code = new CodeDescriptor
{
References = new[] { typeof(IPage).Assembly },
Code = @"
using System.Threading.Tasks;
using Microsoft.Playwright;
class TestClass
{
private IPage _page;
public async Task TestMethod()
{
await _page.QuerySelectorAsync(""#element"");
}
}"
};

await VerifyDiagnosticsAsync(code, new DiagnosticLocation(11, 31));
}

[Test]
public async Task AG0042_WhenUsingQuerySelectorAsyncWithLocalIPageVariable_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 = null;
await page.QuerySelectorAsync(""#element"");
}
}"
};

await VerifyDiagnosticsAsync(code, new DiagnosticLocation(10, 31));
}

[Test]
public async Task AG0042_WhenUsingQuerySelectorAsyncWithIPageProperty_ShowWarning()
{
var code = new CodeDescriptor
{
References = new[] { typeof(IPage).Assembly },
Code = @"
using System.Threading.Tasks;
using Microsoft.Playwright;
class TestClass
{
public IPage Page { get; set; }
public async Task TestMethod()
{
await Page.QuerySelectorAsync(""#element"");
}
}"
};

await VerifyDiagnosticsAsync(code, new DiagnosticLocation(11, 31));
}

[Test]
public async Task AG0042_WhenUsingQuerySelectorAsyncWithNonIPageType_NoWarning()
{
var code = new CodeDescriptor
{
// No need to reference Microsoft.Playwright
Code = @"
using System.Threading.Tasks;
class CustomPage
{
public async Task QuerySelectorAsync(string selector) { }
}
class TestClass
{
public async Task TestMethod()
{
CustomPage page = new CustomPage();
await page.QuerySelectorAsync(""#element"");
}
}"
};

await VerifyDiagnosticsAsync(code, EmptyDiagnosticResults);
}

[Test]
public async Task AG0042_WhenUsingLocatorMethodName_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)
{
page.Locator(""#selector"");
}
}"
};

await VerifyDiagnosticsAsync(code, EmptyDiagnosticResults);
}

[Test]
public async Task AG0042_WhenSymbolIsNull_NoWarning()
{
var code = new CodeDescriptor
{
// Intentionally use an unknown variable to cause symbol to be null
Code = @"
using System.Threading.Tasks;
class TestClass
{
public async Task TestMethod()
{
dynamic unknownVariable = null;
await unknownVariable.QuerySelectorAsync(""#element"");
}
}"
};

await VerifyDiagnosticsAsync(code, EmptyDiagnosticResults);
}

[Test]
public async Task AG0042_WhenTypeSymbolIsNull_NoWarning()
{
var code = new CodeDescriptor
{
Code = @"
using System.Threading.Tasks;
class TestClass
{
public async Task TestMethod(dynamic page)
{
await page.QuerySelectorAsync(""#element"");
}
}"
};

await VerifyDiagnosticsAsync(code, EmptyDiagnosticResults);
}

[Test]
public async Task AG0042_WhenInvocationExpressionIsNotMemberAccess_NoWarning()
{
var code = new CodeDescriptor
{
Code = @"
using System.Threading.Tasks;
class TestClass
{
public async Task TestMethod()
{
await QuerySelectorAsync(""#element"");
}
public async Task QuerySelectorAsync(string selector) { }
}"
};

await VerifyDiagnosticsAsync(code, EmptyDiagnosticResults);
}

[Test]
public async Task AG0042_WhenMemberAccessExpressionHasNoIdentifier_NoWarning()
{
var code = new CodeDescriptor
{
Code = @"
using System.Threading.Tasks;
class TestClass
{
public async Task TestMethod()
{
var func = GetPage();
await func().QuerySelectorAsync(""#element"");
}
public System.Func<dynamic> GetPage() => null;
}"
};

await VerifyDiagnosticsAsync(code, EmptyDiagnosticResults);
}
}
Loading

0 comments on commit 7e9c705

Please sign in to comment.