fix(httpstatuscode): replace spelling-based gating with type-aware detection#42009
Conversation
…tection - Identifier path: resolve type via TypesInfo; for named integer types require both 'http' and 'status' in the type name to avoid false positives (e.g. type JobState int); for plain integers fall back to broadened name list (status, statusCode, httpStatus). - Selector path: accept Status and HTTPStatus field names in addition to StatusCode, fixing false negatives for entry.Status and .HTTPStatus fields. - Adds isHTTPStatusVarName, isHTTPStatusFieldName, isHTTPStatusTypeName helpers. - Extends testdata with: Status/HTTPStatus field cases (flagged), JobState named-enum case (not flagged), and a documented false-negative for non-status-named locals. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…tive cases Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
Great work on this! 🎉 The type-aware detection upgrade to the The PR is tightly scoped to one linter, the description is thorough (the before/after table is especially helpful), and the testdata additions systematically cover every new true-positive and corrected false-positive case. This looks ready for review.
|
PR Triage — Run §28315307719
|
There was a problem hiding this comment.
Pull request overview
This PR tightens the httpstatuscode analyzer’s detection of “HTTP status code context” by moving from spelling-only matching to type-aware checks, reducing false positives/negatives when identifying comparisons/switches against magic HTTP status literals.
Changes:
- Updated identifier handling to consult
pass.TypesInfo.Usesand gate on integer types, using a stricter heuristic for named integer types viaisHTTPStatusTypeName. - Broadened selector/field-name matching to include
StatusandHTTPStatus, and extracted the heuristics into focused helper functions. - Expanded analysistest coverage to exercise the new cases (field names
Status/HTTPStatus, paramhttpStatus, and a non-HTTP named int enum).
Show a summary per file
| File | Description |
|---|---|
| pkg/linters/httpstatuscode/httpstatuscode.go | Reworks status-context detection to be type-aware and expands field-name heuristics via helper functions. |
| pkg/linters/httpstatuscode/testdata/src/httpstatuscode/httpstatuscode.go | Adds test cases for the new identifier/field heuristics and documents an intentional false negative. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
pkg/linters/httpstatuscode/httpstatuscode.go:198
- In the SelectorExpr path, the type-aware guard only checks “is integer”, but it does not apply the named-type heuristic used for *ast.Ident. This means a struct field like
Status JobState(wheretype JobState int) will still be treated as an HTTP status context and get flagged when compared to 200/404, reintroducing the same false-positive class this PR is trying to eliminate for named int enums.
Consider mirroring the ident behavior: after confirming the selected field is an integer, if it’s a *types.Named then require isHTTPStatusTypeName(named.Obj().Name()); otherwise allow plain ints.
if !isHTTPStatusFieldName(e.Sel.Name) {
return false
}
if sel, ok := pass.TypesInfo.Selections[e]; ok {
return isIntegerType(sel.Type())
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Low
isHTTPStatusContextmatched operands by hardcoded name spellings (status,statusCode,.StatusCode) with no type verification on the identifier path — causing false negatives for any other-named HTTP status field/variable and false positives for non-HTTP integers that happen to share a name.Changes
httpstatuscode.gopass.TypesInfo.Uses. For named integer types (type JobState int), requires both"http"and"status"in the type name (case-insensitive) before flagging — prevents false positives onHTTPVersion,HTTPMethod, and unrelated enums. For plainint, falls back to a broadened name list:status,statusCode,httpStatus.StatusandHTTPStatusfield names in addition toStatusCode(with the existing integer-type guard), fixing the silententry.Status == 200class of false negatives.isHTTPStatusVarName,isHTTPStatusFieldName,isHTTPStatusTypeName.Testdata
entry.Status == 200(fieldStatus int)c.HTTPStatus == 404(fieldHTTPStatus int)httpStatus == 200(paramhttpStatus int)state == 200wheretype JobState intcode := resp.StatusCode; code == 404