agoda-com / agoda-com/AgodaAnalyzers

AG0045 false-negative: XPath in interpolated strings (method bodies, Func<> lambdas)

Aberta
#236 0 comentários 0 reações 0 responsáveis Ver no GitHub
Linguagem predominante
C#
Estrelas
25
Forks
15
Métricas de merge de PRs
Nenhum PR com merge em 30d

Descrição

## Summary

AG0045 only inspects `LiteralExpressionSyntax` of kind `StringLiteralExpression` on
`const string` / `static readonly string` field initializers. XPath selectors wrapped
in:

- expression-bodied **method bodies** returning `string`
- **`Func<…,string>` lambda** bodies assigned to `static readonly` fields
- regular method `return` statements with interpolated strings

…are silently accepted. The analyzer gives a false sense of XPath elimination on any
codebase that uses helper methods or lambdas to build selectors — which is a very
common Playwright page-object pattern.

## Reproduction

```csharp
using Microsoft.Playwright;

class BulkUpdatePage
{
// ❌ AG0045 should fire — doesn't
private string SubMenuXPath(string subMenu) => $"//a//*[text()='{subMenu}']";

// ❌ AG0045 should fire — doesn't (Func<,string> lambda body)
private static readonly Func YearXPath =
year => $"//*[@class='ant-picker-content']/tbody//td[@title='{year}']";

// ❌ AG0045 should fire — doesn't (regular method return)
private string DaysCheckboxXPath(string days)
{
return $"//*[@name='selected{days}']/parent::*";
}

// ✅ AG0045 fires correctly
private const string StaticXPath = "//*[@data-selenium='foo']";
}
```

## Why this matters

`IPage.Locator()` in Playwright auto-routes any string beginning with `//` to the
XPath engine. An undetected helper method defeats the entire intent of the rule.

We discovered this during a fleet-wide AG0045 cleanup across ~46 repos. The page
object's diff looked complete, the pipeline passed, but five selectors remained in
XPath at runtime (e.g. `BulkUpdateRatesPage.cs` in a revenue-management acceptance
test suite). The variables were even named `XPath*`, yet the analyzer produced no
diagnostic.

## Suggested detection extension

Extend AG0045 to additionally analyze:

1. **Expression-bodied methods / properties** whose declared return type is `string`
(or assignable to `string`) — inspect the expression for `InterpolatedStringExpressionSyntax`
or `LiteralExpressionSyntax` segments starting with `//` or containing `::`.

2. **Lambda bodies** in field initializers where the inferred/declared return type of
the lambda is `string` — same inspection.

3. **`InterpolatedStringExpressionSyntax`** — walk `InterpolatedStringTextSyntax`
content nodes. If any text token starts with `//` or contains `//` after a
preceding `{…}` interpolation hole, raise AG0045.

4. Optionally: for string-returning methods, consider a lightweight intra-method
data-flow scan (constant propagation of literal segments) to cover
`string.Concat` / `+` chains.

## Related

- Existing false-*positive* issue: #234 (AG0045 fires on filesystem `Path.Combine`
strings not related to Playwright)
- This is the complementary false-*negative* gap.

## Environment

- `Agoda.Analyzers` version: 1.1.185
- Roslyn / C# analyzer host: .NET 8 SDK
- Discovered during: fleet-wide Agoda.Analyzers upgrade (PAO-737)

Guia de contribuição

Abrir o guia de contribuição

Avaliação

Esta issue ainda não foi avaliada.

Receba novas issues na sua caixa de entrada

Um resumo curto de issues do GitHub para quem está começando.