Expand timenowsub coverage to safe selector and index arguments - #46754
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
timenowsub coverage to safe selector and index arguments
There was a problem hiding this comment.
Pull request overview
Note
Copilot could not run the full agentic suite for this review because it was automatically requested on a bot-authored pull request. Request a review from Copilot under Reviewers to retry with the full agentic suite. Improved support for bot-authored pull requests is coming soon.
This PR broadens the timenowsub linter’s notion of “safe” arguments so it can suggest simplifying time.Now().Sub(x) to time.Since(x) for additional argument shapes (selectors and indexing), and updates the linter’s golden test fixtures accordingly. It also refreshes the pinned versions in the generated VulnHunter workflow lockfile.
Changes:
- Extend
isSafeSinceArgto treat selector, index, pointer dereference, and basic literal expressions as “safe”. - Add/adjust
timenowsubtest cases + golden output for selector/index arguments. - Bump versions/pins in
.github/workflows/daily-vulnhunter-scan.lock.yml.
Show a summary per file
| File | Description |
|---|---|
| pkg/linters/timenowsub/timenowsub.go | Expands “safe argument” AST patterns used to decide when to suggest time.Since(...). |
| pkg/linters/timenowsub/testdata/src/timenowsub/timenowsub.go | Adds new test scenarios for selector/index arguments and ensures some call-based variants remain “good”. |
| pkg/linters/timenowsub/testdata/src/timenowsub/timenowsub.go.golden | Updates expected diagnostics/fixes for the new scenarios. |
| .github/workflows/daily-vulnhunter-scan.lock.yml | Updates generated lockfile pins (AWF + Claude Code versions, container digests). |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Low
| case *ast.SelectorExpr: | ||
| return isSafeSinceArg(e.X) | ||
| case *ast.IndexExpr: | ||
| return isSafeSinceArg(e.X) && isSafeSinceArg(e.Index) | ||
| case *ast.StarExpr: | ||
| return isSafeSinceArg(e.X) |
|
🎉 This pull request is included in a new release. Release: |
timenowsubonly rewrotetime.Now().Sub(t)whentwas an ident, so it missed the common stored-start-time formss.startandstarts[i]. This change broadens the safety check to cover side-effect-free selector and index expressions while continuing to exclude call-based arguments.Analyzer coverage
isSafeSinceArgbeyond bare idents to accept pure expression shapes that are evaluation-order invariant aroundtime.Now().Suggested-fix behavior
timequalifier/alias handling unchanged.Regression coverage
badcases for selector and index arguments and corresponding golden rewrites.goodcases proving selector/index expressions that contain calls are still not flagged.Example: