Skip to content

Commit

Permalink
Fix Windows, test it in CI (#961)
Browse files Browse the repository at this point in the history
  • Loading branch information
jakebailey authored Feb 22, 2024
1 parent 40fa6e2 commit 5e7da60
Show file tree
Hide file tree
Showing 10 changed files with 59 additions and 17 deletions.
8 changes: 8 additions & 0 deletions .changeset/twenty-eyes-matter.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
"@definitelytyped/definitions-parser": patch
"@definitelytyped/dtslint-runner": patch
"@definitelytyped/eslint-plugin": patch
"@definitelytyped/utils": patch
---

Fix compatibility with Windows
1 change: 1 addition & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* -text
9 changes: 8 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,14 @@ defaults:
jobs:
build_and_test:
name: build and test
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
os:
- ubuntu-latest
- windows-latest
# - macos-latest # OOMs
runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v2
- uses: actions/setup-node@v2
Expand Down
6 changes: 4 additions & 2 deletions packages/definitions-parser/src/get-affected-packages.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { assertDefined, execAndThrowErrors, mapDefined, normalizeSlashes, withoutStart } from "@definitelytyped/utils";
import { AllPackages, PackageId, formatTypingVersion, getDependencyFromFile } from "./packages";
import { resolve } from "path";
import { isAbsolute } from "path";
import { satisfies } from "semver";
import assert from "assert";
export interface PreparePackagesResult {
readonly packageNames: Set<string>;
readonly dependents: Set<string>;
Expand Down Expand Up @@ -77,7 +78,8 @@ export async function getAffectedPackagesWorker(
dependentOutputs: string[],
definitelyTypedPath: string,
): Promise<PreparePackagesResult> {
const dt = resolve(definitelyTypedPath);
assert(isAbsolute(definitelyTypedPath), "definitelyTypedPath should be absolute");
const dt = definitelyTypedPath;
const changedDirs = mapDefined(changedOutput.split("\n"), getDirectoryName(dt));
const dependentDirs = mapDefined(dependentOutputs.join("\n").split("\n"), getDirectoryName(dt));
const packageNames = new Set([
Expand Down
2 changes: 2 additions & 0 deletions packages/dtslint-runner/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import fs from "fs";
import { RunDTSLintOptions } from "./types";
import { prepareAllPackages } from "./prepareAllPackages";
import { prepareAffectedPackages } from "./prepareAffectedPackages";
import { resolve } from "path";

export async function runDTSLint({
definitelyTypedAcquisition,
Expand Down Expand Up @@ -36,6 +37,7 @@ export async function runDTSLint({
} else {
definitelyTypedPath = definitelyTypedAcquisition.path;
}
definitelyTypedPath = resolve(definitelyTypedPath);

if (!fs.existsSync(definitelyTypedPath)) {
throw new Error(`Path '${definitelyTypedPath}' does not exist.`);
Expand Down
8 changes: 4 additions & 4 deletions packages/eslint-plugin/src/rules/no-bad-reference.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ const rule = createRule({
messageId: "backslashes",
loc: tsRangeToESLintLocation(ref.range),
});
continue;
}

const p = ref.text.startsWith(realNamePlusSlash)
Expand All @@ -88,8 +89,7 @@ const rule = createRule({
)
: ref.text;

const resolved = path.resolve(containingDirectory, p);
const otherPackage = findTypesPackage(resolved);
const otherPackage = findTypesPackage(path.resolve(containingDirectory, p));

if (otherPackage && otherPackage.dir === typesPackage.dir) {
// Perf trick; if a path doesn't have ".." anywhere, then it can't have resolved
Expand All @@ -105,9 +105,9 @@ const rule = createRule({
continue;
}
if (part === "..") {
cwd = path.posix.dirname(cwd);
cwd = path.dirname(cwd);
} else {
cwd = path.posix.join(cwd, part);
cwd = path.join(cwd, part);
}
const otherPackage = findTypesPackage(cwd);
if (otherPackage && otherPackage.dir === typesPackage.dir) {
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export function findUp<T extends {}>(p: string, fn: (p: string) => T | undefined
}

export interface TypesPackageInfo {
/** directory containing package json; this is a OS-specific path */
dir: string;
/** package.json with name="@types/foo__bar-baz" */
packageJson: PackageJSON;
Expand Down
17 changes: 15 additions & 2 deletions packages/eslint-plugin/test/eslint.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { fixtureRoot } from "./util";
import { toMatchFile } from "jest-file-snapshot";
import * as plugin from "../src/index";
import fs from "fs";
import { normalizeSlashes } from "@definitelytyped/utils";

expect.extend({ toMatchFile });
const snapshotDir = path.join(__dirname, "__file_snapshots__");
Expand Down Expand Up @@ -50,11 +51,23 @@ for (const fixture of allFixtures) {
const resultText = stripAnsi(formatted).trim() || "No errors";
expect(resultText).not.toContain("Parsing error");
const newOutput = formatResultsWithInlineErrors(results);
expect(resultText + "\n\n" + newOutput).toMatchFile(getLintSnapshotPath(fixture));
expect(normalizeSnapshot(resultText + "\n\n" + newOutput)).toMatchFile(getLintSnapshotPath(fixture));
});
});
}

function normalizeSnapshot(snapshot: string): string {
return snapshot
.split(/\r?\n/g)
.map((line) => {
if (line.startsWith("types\\")) {
return normalizeSlashes(line);
}
return line;
})
.join("\n");
}

function formatResultsWithInlineErrors(results: ESLint.LintResult[]): string {
const output: string[] = [];

Expand All @@ -74,7 +87,7 @@ function formatResultsWithInlineErrors(results: ESLint.LintResult[]): string {
const indent = " ";

for (const result of results) {
output.push(`==== ${result.filePath} ====`);
output.push(`==== ${normalizeSlashes(result.filePath)} ====`);
output.push("");

const sourceText = fs.readFileSync(path.join(fixtureRoot, result.filePath), "utf-8");
Expand Down
12 changes: 6 additions & 6 deletions packages/utils/src/fs.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import assert from "assert";
import { join, relative, resolve, isAbsolute } from "path";
import systemPath, { posix as posixPath } from "path";
import getCacheDir = require("cachedir");
import { assertDefined } from "./assertions";
import fs from "fs";
import { readFileSync, readJsonSync } from "./io";

/** The directory to read/write suggestsions from */
export const suggestionsDir = join(getCacheDir("dts"), "suggestions");
export const suggestionsDir = systemPath.join(getCacheDir("dts"), "suggestions");

/** Convert a path to use "/" instead of "\\" for consistency. (This affects content hash.) */
export function normalizeSlashes(path: string): string {
Expand Down Expand Up @@ -91,7 +91,7 @@ export class InMemoryFS implements FS {

private tryGetEntry(path: string): ReadonlyDir | string | undefined {
if (path[0] === "/") {
path = relative(this.rootPrefix, path);
path = posixPath.relative(this.rootPrefix, path);
}
if (path === "") {
return this.curDir;
Expand Down Expand Up @@ -162,7 +162,7 @@ export class InMemoryFS implements FS {

subDir(path: string): FS {
assert(path[0] !== "/", "Cannot use absolute paths with InMemoryFS.subDir");
return new InMemoryFS(this.getDir(path), resolve(this.rootPrefix, path));
return new InMemoryFS(this.getDir(path), posixPath.join(this.rootPrefix, path));
}

debugPath(): string {
Expand All @@ -179,12 +179,12 @@ export class InMemoryFS implements FS {

export class DiskFS implements FS {
constructor(private readonly rootPrefix: string) {
assert(isAbsolute(rootPrefix), "DiskFS must use absolute paths");
assert(systemPath.isAbsolute(rootPrefix), "DiskFS must use absolute paths");
this.rootPrefix = ensureTrailingSlash(rootPrefix);
}

private getPath(path: string | undefined): string {
return resolve(this.rootPrefix, path ?? "");
return systemPath.resolve(this.rootPrefix, path ?? "");
}

readdir(dirPath?: string): readonly string[] {
Expand Down
12 changes: 10 additions & 2 deletions tsconfig.test.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,17 @@
},
"files": [],
"references": [
{ "path": "packages/header-parser/test" },
{ "path": "packages/definitions-parser/test" },
{ "path": "packages/dts-critic" },
{ "path": "packages/dts-gen" },
{ "path": "packages/dtslint/test" },
{ "path": "packages/dtslint-runner" },
{ "path": "packages/eslint-plugin/test" },
{ "path": "packages/header-parser/test" },
{ "path": "packages/publisher/test" },
{ "path": "packages/utils/test" }
{ "path": "packages/retag" },
{ "path": "packages/typescript-packages/test" },
{ "path": "packages/typescript-versions/test" },
{ "path": "packages/utils/test" },
]
}

0 comments on commit 5e7da60

Please sign in to comment.