diff --git a/packages/pyright-internal/src/analyzer/checker.ts b/packages/pyright-internal/src/analyzer/checker.ts index 5037e3b31de0..60e337877d3d 100644 --- a/packages/pyright-internal/src/analyzer/checker.ts +++ b/packages/pyright-internal/src/analyzer/checker.ts @@ -1628,6 +1628,19 @@ export class Checker extends ParseTreeWalker { } } + if ( + reportAsUnused && + this._fileInfo.ipythonMode === IPythonMode.CellDocs && + node.parent?.nodeType === ParseNodeType.StatementList && + node.parent.statements[node.parent.statements.length - 1] === node && + node.parent.parent?.nodeType === ParseNodeType.Module && + node.parent.parent.statements[node.parent.parent.statements.length - 1] === node.parent + ) { + // Exclude an expression at the end of a notebook cell, as that is treated as + // the cell's value. + reportAsUnused = false; + } + if (reportAsUnused) { this._evaluator.addDiagnostic( this._fileInfo.diagnosticRuleSet.reportUnusedExpression, diff --git a/packages/pyright-internal/src/analyzer/sourceFile.ts b/packages/pyright-internal/src/analyzer/sourceFile.ts index 68cc6348e9d8..e0864723b4a5 100644 --- a/packages/pyright-internal/src/analyzer/sourceFile.ts +++ b/packages/pyright-internal/src/analyzer/sourceFile.ts @@ -1365,7 +1365,7 @@ export class SourceFile { } test_enableIPythonMode(enable: boolean) { - this._ipythonMode = enable ? IPythonMode.ConcatDoc : IPythonMode.None; + this._ipythonMode = enable ? IPythonMode.CellDocs : IPythonMode.None; } private _buildFileInfo( diff --git a/packages/pyright-internal/src/tests/harness/fourslash/testState.ts b/packages/pyright-internal/src/tests/harness/fourslash/testState.ts index edbf61fde2aa..a1ab3f9465a7 100644 --- a/packages/pyright-internal/src/tests/harness/fourslash/testState.ts +++ b/packages/pyright-internal/src/tests/harness/fourslash/testState.ts @@ -521,7 +521,7 @@ export class TestState { } verifyDiagnostics(map?: { [marker: string]: { category: string; message: string } }): void { - this._analyze(); + this.analyze(); // organize things per file const resultPerFile = this._getDiagnosticsPerFile(); @@ -647,7 +647,7 @@ export class TestState { ): Promise { // make sure we don't use cache built from other tests this.workspace.serviceInstance.invalidateAndForceReanalysis(); - this._analyze(); + this.analyze(); for (const range of this.getRanges()) { const name = this.getMarkerName(range.marker!); @@ -713,7 +713,7 @@ export class TestState { } async verifyCommand(command: Command, files: { [filePath: string]: string }): Promise { - this._analyze(); + this.analyze(); const commandResult = await this._hostSpecificFeatures.execute( new TestLanguageService(this.workspace, this.console, this.fs), @@ -751,7 +751,7 @@ export class TestState { }, verifyCodeActionCount?: boolean ): Promise { - this._analyze(); + this.analyze(); for (const range of this.getRanges()) { const name = this.getMarkerName(range.marker!); @@ -904,7 +904,7 @@ export class TestState { }, abbrMap?: { [abbr: string]: AbbreviationInfo } ): Promise { - this._analyze(); + this.analyze(); for (const marker of this.getMarkers()) { const markerName = this.getMarkerName(marker); @@ -1074,7 +1074,7 @@ export class TestState { }; } ): void { - this._analyze(); + this.analyze(); for (const marker of this.getMarkers()) { const fileName = marker.fileName; @@ -1143,7 +1143,7 @@ export class TestState { references: DocumentRange[]; }; }) { - this._analyze(); + this.analyze(); for (const marker of this.getMarkers()) { const fileName = marker.fileName; @@ -1193,7 +1193,7 @@ export class TestState { references: DocumentHighlight[]; }; }) { - this._analyze(); + this.analyze(); for (const name of Object.keys(map)) { const marker = this.getMarkerByName(name); @@ -1225,7 +1225,7 @@ export class TestState { }, filter: DefinitionFilter = DefinitionFilter.All ) { - this._analyze(); + this.analyze(); for (const marker of this.getMarkers()) { const fileName = marker.fileName; @@ -1253,7 +1253,7 @@ export class TestState { definitions: DocumentRange[]; }; }) { - this._analyze(); + this.analyze(); for (const marker of this.getMarkers()) { const fileName = marker.fileName; @@ -1282,7 +1282,7 @@ export class TestState { changes: FileEditAction[]; }; }) { - this._analyze(); + this.analyze(); for (const marker of this.getMarkers()) { const fileName = marker.fileName; @@ -1626,7 +1626,7 @@ export class TestState { return position <= editStart ? position : position < editEnd ? -1 : position + length - +(editEnd - editStart); } - private _analyze() { + public analyze() { while (this.program.analyze()) { // Continue to call analyze until it completes. Since we're not // specifying a timeout, it should complete the first time. diff --git a/packages/pyright-internal/src/tests/ipythonMode.test.ts b/packages/pyright-internal/src/tests/ipythonMode.test.ts index 85d831b33988..6e9631940257 100644 --- a/packages/pyright-internal/src/tests/ipythonMode.test.ts +++ b/packages/pyright-internal/src/tests/ipythonMode.test.ts @@ -9,6 +9,7 @@ import assert from 'assert'; import { CompletionItemKind, MarkupKind } from 'vscode-languageserver-types'; +import { DiagnosticRule } from '../common/diagnosticRules'; import { TextRange } from '../common/textRange'; import { TextRangeCollection } from '../common/textRangeCollection'; import { Localizer } from '../localization/localize'; @@ -560,3 +561,51 @@ function findCommentByOffset(tokens: TextRangeCollection, offset: number) return comment; } + +test('unused expression at end is not error', async () => { + const code = ` +// @filename: test.py +// @ipythonMode: true +//// 4[|/*marker*/|] + `; + + verifyAnalysisDiagnosticCount(code, 0); +}); + +test('unused expression is error if not at end of cell', async () => { + const code = ` +// @filename: test.py +// @ipythonMode: true +//// 4[|/*marker*/|] +//// +//// x = 1 + `; + + verifyAnalysisDiagnosticCount(code, 1, DiagnosticRule.reportUnusedExpression); +}); + +test('unused expression is error if within another statement', async () => { + const code = ` +// @filename: test.py +// @ipythonMode: true +//// if True: +//// 4[|/*marker*/|] + `; + + verifyAnalysisDiagnosticCount(code, 1, DiagnosticRule.reportUnusedExpression); +}); + +function verifyAnalysisDiagnosticCount(code: string, expectedCount: number, expectedRule?: string) { + const state = parseAndGetTestState(code).state; + + state.analyze(); + + const range = state.getRangeByMarkerName('marker')!; + const source = state.program.getBoundSourceFile(range.fileName)!; + const diagnostics = source.getDiagnostics(state.configOptions); + + assert.strictEqual(diagnostics?.length, expectedCount); + if (expectedRule) { + diagnostics.forEach((diagnostic) => assert.strictEqual(diagnostic.getRule(), expectedRule)); + } +}