Skip to content

Commit bdd47e1

Browse files
varungandhi-srcvarungandhi-src
committed
Add assertions for comments related to path casing
1 parent 2304a1b commit bdd47e1

File tree

4 files changed

+175
-9
lines changed

4 files changed

+175
-9
lines changed

‎packages/pyright-scip/src/assertions.ts‎

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
import { normalizePathCase, isFileSystemCaseSensitive } from 'pyright-internal/common/pathUtils';
2+
import { PyrightFileSystem } from 'pyright-internal/pyrightFileSystem';
3+
import { createFromRealFileSystem } from 'pyright-internal/common/realFileSystem';
4+
5+
export enum SeenCondition {
6+
AlwaysFalse = 'always-false',
7+
AlwaysTrue = 'always-true',
8+
Mixed = 'mixed'
9+
}
10+
11+
export class AssertionError extends Error {
12+
constructor(message: string) {
13+
super(message);
14+
this.name = 'AssertionError';
15+
}
16+
}
17+
18+
// Private global state - never export directly
19+
let _assertionFlags = {
20+
pathNormalizationChecks: false,
21+
otherChecks: false
22+
};
23+
let _context = '';
24+
const _sometimesResults = new Map<string, Map<string, SeenCondition>>();
25+
26+
export function setGlobalAssertionFlags(pathNormalizationChecks: boolean, otherChecks: boolean): void {
27+
_assertionFlags.pathNormalizationChecks = pathNormalizationChecks;
28+
_assertionFlags.otherChecks = otherChecks;
29+
}
30+
31+
export function setGlobalContext(context: string): void {
32+
_context = context;
33+
}
34+
35+
// Internal implementation functions
36+
function assertAlwaysImpl(enableFlag: boolean, check: () => boolean, message: () => string): void {
37+
if (!enableFlag) return;
38+
39+
if (!check()) {
40+
throw new AssertionError(message());
41+
}
42+
}
43+
44+
function assertSometimesImpl(enableFlag: boolean, check: () => boolean, key: string): void {
45+
if (!enableFlag) return;
46+
47+
const ctx = _context;
48+
if (ctx === '') {
49+
throw new AssertionError('Context must be set before calling assertSometimes');
50+
}
51+
52+
let ctxMap = _sometimesResults.get(key);
53+
if (!ctxMap) {
54+
ctxMap = new Map();
55+
_sometimesResults.set(key, ctxMap);
56+
}
57+
58+
const result = check() ? SeenCondition.AlwaysTrue : SeenCondition.AlwaysFalse;
59+
const prev = ctxMap.get(ctx);
60+
61+
if (prev === undefined) {
62+
ctxMap.set(ctx, result);
63+
} else if (prev !== result) {
64+
ctxMap.set(ctx, SeenCondition.Mixed);
65+
}
66+
}
67+
68+
const _fs = new PyrightFileSystem(createFromRealFileSystem());
69+
70+
export function assertAlways(check: () => boolean, message: () => string): void {
71+
assertAlwaysImpl(_assertionFlags.otherChecks, check, message);
72+
}
73+
74+
export function assertSometimes(check: () => boolean, key: string): void {
75+
assertSometimesImpl(_assertionFlags.otherChecks, check, key);
76+
}
77+
78+
export function assertNeverNormalized(path: string): void {
79+
const normalized = normalizePathCase(_fs, path);
80+
assertAlwaysImpl(
81+
_assertionFlags.pathNormalizationChecks,
82+
() => normalized !== path,
83+
() => `Path should not be normalized but was: ${path}`
84+
);
85+
}
86+
87+
export function assertAlwaysNormalized(path: string): void {
88+
const normalized = normalizePathCase(_fs, path);
89+
assertAlwaysImpl(
90+
_assertionFlags.pathNormalizationChecks,
91+
() => normalized === path,
92+
() => `Path should be normalized but was not: ${path} -> ${normalized}`
93+
);
94+
}
95+
96+
export function assertSometimesNormalized(path: string, key: string): void {
97+
const normalized = normalizePathCase(_fs, path);
98+
assertSometimesImpl(
99+
_assertionFlags.pathNormalizationChecks,
100+
() => normalized === path,
101+
key
102+
);
103+
}
104+
105+
// Monoidal combination logic
106+
function combine(a: SeenCondition, b: SeenCondition): SeenCondition {
107+
if (a === b) return a;
108+
if (a === SeenCondition.Mixed || b === SeenCondition.Mixed) {
109+
return SeenCondition.Mixed;
110+
}
111+
// AlwaysTrue + AlwaysFalse = Mixed
112+
return SeenCondition.Mixed;
113+
}
114+
115+
export function checkSometimesAssertions(): Map<string, SeenCondition> {
116+
const summary = new Map<string, SeenCondition>();
117+
118+
for (const [key, ctxMap] of _sometimesResults) {
119+
let agg: SeenCondition | undefined;
120+
for (const state of ctxMap.values()) {
121+
agg = agg === undefined ? state : combine(agg, state);
122+
if (agg === SeenCondition.Mixed) break;
123+
}
124+
if (agg !== undefined) {
125+
summary.set(key, agg);
126+
}
127+
}
128+
129+
return summary;
130+
}

‎packages/pyright-scip/src/test-runner.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
import * as fs from 'fs';
22
import * as path from 'path';
33
import { join } from 'path';
4+
import { checkSometimesAssertions, SeenCondition } from './assertions';
45

56
export interface TestFailure {
67
testName: string;
7-
type: 'empty-scip-index' | 'missing-output' | 'content-mismatch' | 'orphaned-output' | 'caught-exception';
8+
type: 'empty-scip-index' | 'missing-output' | 'content-mismatch' | 'orphaned-output' | 'caught-exception' | 'sometimes-assertion';
89
message: string;
910
}
1011

@@ -157,6 +158,17 @@ export class TestRunner {
157158
}
158159
}
159160

161+
const sometimesResults = checkSometimesAssertions();
162+
for (const [key, state] of sometimesResults) {
163+
if (state === SeenCondition.Mixed) continue; // success
164+
165+
results.failed.push({
166+
testName: 'assertions',
167+
type: 'sometimes-assertion',
168+
message: `Assertion '${key}' was ${state} across all test contexts`
169+
});
170+
}
171+
160172
reportResults(results);
161173
}
162174
}

‎packages/pyright-scip/src/treeVisitor.ts‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,10 @@ import { HoverResults } from 'pyright-internal/languageService/hoverProvider';
5454
import { convertDocStringToMarkdown } from 'pyright-internal/analyzer/docStringConversion';
5555
import { assert } from 'pyright-internal/common/debug';
5656
import { getClassFieldsRecursive } from 'pyright-internal/analyzer/typeUtils';
57-
import {createFromFileSystem} from "pyright-internal/tests/harness/vfs/factory";
58-
import {PyrightFileSystem} from "pyright-internal/pyrightFileSystem";
59-
import {createFromRealFileSystem} from "pyright-internal/common/realFileSystem";
60-
import {normalizePathCase} from "pyright-internal/common/pathUtils";
57+
import { PyrightFileSystem } from "pyright-internal/pyrightFileSystem";
58+
import { createFromRealFileSystem } from "pyright-internal/common/realFileSystem";
59+
import { normalizePathCase } from "pyright-internal/common/pathUtils";
60+
import { assertNeverNormalized, assertSometimesNormalized } from "./assertions";
6161

6262
// Useful functions for later, but haven't gotten far enough yet to use them.
6363
// extractParameterDocumentation
@@ -514,6 +514,8 @@ export class TreeVisitor extends ParseTreeWalker {
514514
// If we remove one of the two checks below, existing tests start failing
515515
// (aliased_import and nested_items tests). So do both checks.
516516
const resolvedPath = path.resolve(importInfo.resolvedPaths[0])
517+
assertSometimesNormalized(resolvedPath, 'visitImportAs.resolvedPath')
518+
517519
return resolvedPath.startsWith(this.cwd) ||
518520
resolvedPath.startsWith(
519521
normalizePathCase(new PyrightFileSystem(createFromRealFileSystem()), this.cwd))
@@ -1430,11 +1432,13 @@ export class TreeVisitor extends ParseTreeWalker {
14301432
const nodeFilePath = path.resolve(nodeFileInfo.filePath);
14311433

14321434
// TODO: Should use files from the package to determine this -- should be able to do that quite easily.
1435+
1436+
// NOTE: Unlike other code paths where we have
1437+
// HACK(id: inconsistent-casing-of-resolved-paths),
1438+
// here, nodeFilePath seems to never be normalized,
1439+
// so avoid a separate check.
1440+
assertNeverNormalized(nodeFilePath);
14331441
if (nodeFilePath.startsWith(this.cwd)) {
1434-
// NOTE: Unlike other code paths where we have
1435-
// HACK(id: inconsistent-casing-of-resolved-paths),
1436-
// here, nodeFilePath seems to never be normalized,
1437-
// so avoid a separate check.
14381442
return this.projectPackage;
14391443
}
14401444

@@ -1658,6 +1662,7 @@ export class TreeVisitor extends ParseTreeWalker {
16581662
// On a case-insensitive filesystem, p can sometimes be fully lowercased
16591663
// (e.g. see the nested_items test), and sometimes it may have uppercase
16601664
// characters (e.g. the unique test).
1665+
assertSometimesNormalized(p, 'guessPackage.declPath.resolved');
16611666
if (p.startsWith(this.cwd) ||
16621667
p.startsWith(normalizePathCase(new PyrightFileSystem(createFromRealFileSystem()), this.cwd))) {
16631668
return this.projectPackage;

‎packages/pyright-scip/test/test-main.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,15 @@ import { join } from 'path';
88
import * as path from 'path';
99
import * as fs from 'fs';
1010
import { Indexer } from '../src/indexer';
11+
import {
12+
setGlobalAssertionFlags,
13+
setGlobalContext,
14+
checkSometimesAssertions,
15+
SeenCondition
16+
} from '../src/assertions';
17+
import { normalizePathCase, isFileSystemCaseSensitive } from 'pyright-internal/common/pathUtils';
18+
import { PyrightFileSystem } from 'pyright-internal/pyrightFileSystem';
19+
import { createFromRealFileSystem } from 'pyright-internal/common/realFileSystem';
1120

1221
function createTempDirectory(outputDirectory: string, testName: string): string {
1322
const tempPrefix = path.join(path.dirname(outputDirectory), `.tmp-${testName}-`);
@@ -271,6 +280,13 @@ function unitTests(): void {
271280

272281
function snapshotTests(mode: 'check' | 'update', failFast: boolean, quiet: boolean, filterTests?: string[]): void {
273282
const snapshotRoot = './snapshots';
283+
const cwd = process.cwd();
284+
285+
// Initialize assertion flags
286+
const fileSystem = new PyrightFileSystem(createFromRealFileSystem());
287+
const pathNormalizationChecks = !isFileSystemCaseSensitive(fileSystem) && normalizePathCase(fileSystem, cwd) !== cwd;
288+
const otherChecks = true;
289+
setGlobalAssertionFlags(pathNormalizationChecks, otherChecks);
274290

275291
// Load package info to determine project name and version per test
276292
const packageInfoPath = path.join(snapshotRoot, 'packageInfo.json');
@@ -285,6 +301,9 @@ function snapshotTests(mode: 'check' | 'update', failFast: boolean, quiet: boole
285301
});
286302

287303
testRunner.runTests((testName, inputDir, outputDir) => {
304+
// Set context for this test
305+
setGlobalContext(testName);
306+
288307
let projectName: string | undefined;
289308
let projectVersion: string | undefined;
290309

0 commit comments

Comments
 (0)