From cafa689a51500040ad0c1e6f02cf214945224e2e Mon Sep 17 00:00:00 2001 From: FROWNINGdev <218313741+FROWNINGdev@users.noreply.github.com> Date: Mon, 28 Sep 2026 01:22:00 +0500 Subject: [PATCH] fix(diagnostics): re-lint open files on each scan; read nested DOL007 loop heads Files open when VS Code starts were linted before the first workspace scan finished and never again, so DOL008 and the schema gates of DOL007 stayed silent until the user typed in them; a models.py edit likewise never reached views already open. registerCodeFixes now takes an index-changed event and re-lints every open document when it fires. DOL007's loop-head regex allowed only flat argument lists and no call after the first, so for o in Order.objects.filter(created__lte=timezone.now()): and for o in Order.objects.filter(...).order_by("-id"): were never inspected. The head is now parsed with balanced parentheses across the whole chain, skipping string literals. --- CHANGELOG.md | 18 ++++++++++++ src/codeActionProvider.ts | 15 ++++++++-- src/extension.ts | 5 +++- src/rules/queryset.ts | 55 +++++++++++++++++++++++++++++++---- test/rules/n-plus-one.test.js | 37 +++++++++++++++++++++++ 5 files changed, 121 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c97600..8a55a98 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,24 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- **Schema-aware diagnostics were missing in files open at startup.** Every + document open when VS Code starts is linted before the first workspace scan + finishes, and nothing re-linted it afterwards, so `DOL008` (misspelled + lookup) and the schema gates of `DOL007` stayed silent until the file was + edited. The same gap meant a `models.py` change never reached diagnostics in + views that were already open. Open documents are now re-linted whenever the + index is replaced. + +- **`DOL007` skipped loops over common querysets.** The loop-head pattern only + accepted flat argument lists and no call after the first, so + `for o in Order.objects.filter(created__lte=timezone.now()):` and + `for o in Order.objects.filter(...).order_by("-id"):` were never inspected. + The head is now read with balanced parentheses (string literals skipped) + across the whole call chain, and a `select_related(...)` anywhere in that + chain still silences the finding. + ## [0.19.0] - 2026-09-21 ### Added diff --git a/src/codeActionProvider.ts b/src/codeActionProvider.ts index 6f25167..7307df9 100644 --- a/src/codeActionProvider.ts +++ b/src/codeActionProvider.ts @@ -182,6 +182,11 @@ export function registerCodeFixes( // lens providers take it: the index is replaced wholesale on every scan, and // a captured value would go stale the first time a models.py is saved. getIndex?: () => WorkspaceIndex, + // Schema-aware rules (DOL007's gates, DOL008) are silent without an index, + // and documents open at startup are linted before the first scan finishes. + // Without a re-lint here they stay silent until the user types in them, and + // a models.py edit never reaches diagnostics in the views already open. + onIndexChanged?: vscode.Event, ): vscode.Disposable[] { const collection = vscode.languages.createDiagnosticCollection('djangoOrmLens'); const provider = new DjangoCodeFixesProvider(collection, getIndex); @@ -217,9 +222,13 @@ export function registerCodeFixes( }), ]; - for (const doc of vscode.workspace.textDocuments) { - refreshDiagnosticsForDoc(doc, collection, getIndex); - } + const refreshAll = (): void => { + for (const doc of vscode.workspace.textDocuments) { + refreshDiagnosticsForDoc(doc, collection, getIndex); + } + }; + if (onIndexChanged) disposables.push(onIndexChanged(refreshAll)); + refreshAll(); return disposables; } diff --git a/src/extension.ts b/src/extension.ts index 3cae244..33adeee 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -27,6 +27,7 @@ import { completionsAt, lookupPathLength } from './ormCompletions'; const LOOKBACK_LINES = 200; let currentIndex: WorkspaceIndex = { apps: [], scannedAt: 0 }; +const indexChanged = new vscode.EventEmitter(); let treeProvider: DjangoTreeProvider; let codeLensProvider: DjangoCodeLensProvider; let statusItem: vscode.StatusBarItem; @@ -58,6 +59,7 @@ async function doScan(): Promise { currentIndex = next; treeProvider.setIndex(currentIndex); codeLensProvider?.refresh(); + indexChanged.fire(); const total = currentIndex.apps.reduce((n, a) => n + a.models.length, 0); const took = Date.now() - before; statusItem.text = `$(database) ${total} model${total === 1 ? '' : 's'}`; @@ -323,7 +325,8 @@ export async function activate(context: vscode.ExtensionContext) { // `.count() > 0` -> `.exists()`, `.first() is None` -> `not .exists()`, // and N+1 attribute-access-in-loop diagnostics. Feature-flagged via // djangoOrmLens.codeFixes.enabled (default true). - for (const d of registerCodeFixes(context, () => currentIndex)) { + context.subscriptions.push(indexChanged); + for (const d of registerCodeFixes(context, () => currentIndex, indexChanged.event)) { context.subscriptions.push(d); } diff --git a/src/rules/queryset.ts b/src/rules/queryset.ts index c360969..305ba28 100644 --- a/src/rules/queryset.ts +++ b/src/rules/queryset.ts @@ -33,8 +33,7 @@ const RE_FILTER_EXCLUDE_CHAIN = /(\b[A-Za-z_][\w.]*(?:\([^()]*\))*)\.filter\(([^()]*)\)\s*\.exclude\(([^()]*)\)/g; const RE_LIST_OF_QS_IN_FOR = /^\s*for\s+[A-Za-z_]\w*\s+in\s+list\(([A-Za-z_][\w.]*(?:\([^()]*\))*)\)\s*:/; -const RE_FOR_LOOP_HEAD = - /^\s*for\s+([A-Za-z_]\w*)\s+in\s+([A-Za-z_][\w.]*(?:\([^()]*\))*(?:\.all\(\))?)\s*:/; +const RE_FOR_LOOP_START = /^\s*for\s+([A-Za-z_]\w*)\s+in\s+/; const LOOP_VAR_ATTR_RE = /\b([A-Za-z_]\w*)\.([A-Za-z_]\w*)\b/g; const MAX_LOOP_LINES = 15; @@ -90,6 +89,52 @@ function isCommentLine(text: string): boolean { * says nothing about what comes back and a line-oriented rule cannot * follow a callee's return. */ +/** Index of the `)` closing the `(` at `open`, skipping string literals; -1 if unclosed. */ +function closingParen(text: string, open: number): number { + let depth = 0; + let quote: string | null = null; + for (let i = open; i < text.length; i++) { + const c = text[i]; + if (quote) { + if (c === '\\') i++; + else if (c === quote) quote = null; + } else if (c === '"' || c === "'") { + quote = c; + } else if (c === '(') { + depth++; + } else if (c === ')' && --depth === 0) { + return i; + } + } + return -1; +} + +/** + * `for in :` where `` is a dotted chain whose calls may + * nest parentheses and follow one another: + * `Order.objects.filter(created__lte=now()).order_by("-id")`. + */ +function parseLoopHead(line: string): { loopVar: string; source: string } | null { + const head = RE_FOR_LOOP_START.exec(line); + if (!head) return null; + const start = head[0].length; + let i = start; + for (;;) { + const name = /^[A-Za-z_]\w*/.exec(line.slice(i)); + if (!name) return null; + i += name[0].length; + while (line[i] === '(') { + const end = closingParen(line, i); + if (end < 0) return null; + i = end + 1; + } + if (line[i] !== '.') break; + i++; + } + if (!/^\s*:/.test(line.slice(i))) return null; + return { loopVar: head[1], source: line.slice(start, i) }; +} + function mayIterateQuerySet(expr: string): boolean { const callAt = expr.indexOf('('); if (callAt === -1) return true; @@ -360,10 +405,10 @@ const DOL007: Rule = { check(ctx: RuleContext): Finding[] { const out: Finding[] = []; for (let i = 0; i < ctx.lineCount; i++) { - const head = RE_FOR_LOOP_HEAD.exec(ctx.lineAt(i)); + const head = parseLoopHead(ctx.lineAt(i)); if (!head) continue; - const loopVar = head[1]; - const qsExpr = head[2]; + const loopVar = head.loopVar; + const qsExpr = head.source; if (!mayIterateQuerySet(qsExpr)) continue; // The chain that produced the loop's source is usually on an earlier // line (`codes = list(Model.objects.select_related("x"))`), so both the diff --git a/test/rules/n-plus-one.test.js b/test/rules/n-plus-one.test.js index 265cfae..b85b44c 100644 --- a/test/rules/n-plus-one.test.js +++ b/test/rules/n-plus-one.test.js @@ -243,3 +243,40 @@ test('DOL007 does not follow a binding across a scope boundary', () => { assert.equal(findings.length, 1); assert.equal(findings[0].fixHint, 'user'); }); + +test('DOL007 reads a loop head whose filter nests a call', () => { + // `created__lte=timezone.now()` put a paren pair inside the call, and the + // loop-head pattern only allowed flat argument lists, so the most common + // shape of a date-bounded report loop was never looked at. + const rule = ruleByCode('DOL007'); + const src = [ + 'def report():', + ' for m in Membership.objects.filter(created__lte=timezone.now()):', + ' print(m.roles, m.user.email)', + ].join('\n'); + const findings = rule.check(makeCtx(src, REPORTER_INDEX)); + assert.equal(findings.length, 1); + assert.equal(findings[0].fixHint, 'user'); +}); + +test('DOL007 reads a loop head that chains calls after the first one', () => { + const rule = ruleByCode('DOL007'); + const src = [ + 'def report():', + ' for m in Membership.objects.filter(roles="admin").order_by("-id"):', + ' print(m.user)', + ].join('\n'); + const findings = rule.check(makeCtx(src, REPORTER_INDEX)); + assert.equal(findings.length, 1); + assert.equal(findings[0].fixHint, 'user'); +}); + +test('DOL007 honours select_related later in a chained loop head', () => { + const rule = ruleByCode('DOL007'); + const src = [ + 'def report():', + ' for m in Membership.objects.filter(roles="a)b").select_related("user"):', + ' print(m.user)', + ].join('\n'); + assert.equal(rule.check(makeCtx(src, REPORTER_INDEX)).length, 0); +});