Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Windows, test it in CI #961

Merged
merged 16 commits into from
Feb 22, 2024
Merged
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" },
]
}
Loading