Skip to content

Commit

Permalink
fix: remove unused support for file glob from command line (#1534)
Browse files Browse the repository at this point in the history
## PR Checklist

- [x] Addresses an existing open issue: fixes #1533
- [x] That issue was marked as [`status: accepting
prs`](https://github.com/JoshuaKGoldberg/TypeStat/issues?q=is%3Aopen+is%3Aissue+label%3A%22status%3A+accepting+prs%22)
- [x] Steps in
[CONTRIBUTING.md](https://github.com/JoshuaKGoldberg/TypeStat/blob/main/.github/CONTRIBUTING.md)
were taken

## Overview

There seems to be undocumented code path that could potentially support
giving file glob from command line. Since there is no tests for this and
no documentation, it's hard to say is it working as intended currently.
I think the current mindset is that these kind of settings would be in
the json config file.

This change makes the code simpler to understand.

---------

Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com>
  • Loading branch information
rubiesonthesky and JoshuaKGoldberg authored Nov 29, 2024
1 parent ac67a7c commit f21114e
Show file tree
Hide file tree
Showing 7 changed files with 33 additions and 26 deletions.
2 changes: 1 addition & 1 deletion src/cli/runCli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export const runCli = async (
let result: TypeStatResult;

try {
result = await runtime.mainRunner(rawOptions, runtime.output);
result = await runtime.mainRunner(rawOptions.config, runtime.output);
} catch (error) {
result = {
error: error instanceof Error ? error : new Error(error as string),
Expand Down
23 changes: 23 additions & 0 deletions src/collectFileNames.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import path from "node:path";
import { describe, expect, it } from "vitest";

import { collectFileNames } from "./collectFileNames.js";

describe("collectFileNames", () => {
it("should collect files with wildcard when collection succeeds", async () => {
const cwd = path.resolve(import.meta.dirname, "..");
const fileNames = await collectFileNames(
path.resolve(import.meta.dirname),
["*"],
);
expect(fileNames).toContain(`${cwd}/src/collectFileNames.test.ts`);
});

it("should return error if node_modules are implicitly included", async () => {
const cwd = path.resolve(import.meta.dirname, "..");
const fileNames = await collectFileNames(cwd, ["*"]);
expect(fileNames).toEqual(
`At least one path including node_modules was included implicitly: '${cwd}/node_modules'. Either adjust TypeStat's included files to not include node_modules (recommended) or explicitly include node_modules/ (not recommended).`,
);
});
});
10 changes: 1 addition & 9 deletions src/collectFileNames.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
import { glob } from "glob";
import * as path from "node:path";

import { TypeStatArgv } from "./index.js";

export const collectFileNames = async (
argv: TypeStatArgv,
cwd: string,
include: readonly string[] | undefined,
): Promise<readonly string[] | string | undefined> => {
const globsAndNames = await collectFileNamesFromGlobs(argv, cwd, include);
const globsAndNames = await collectFileNamesFromGlobs(cwd, include);
if (!globsAndNames) {
return undefined;
}
Expand All @@ -27,14 +24,9 @@ export const collectFileNames = async (
};

const collectFileNamesFromGlobs = async (
argv: TypeStatArgv,
cwd: string,
include: readonly string[] | undefined,
): Promise<[readonly string[], readonly string[]] | undefined> => {
if (argv.args.length) {
return [argv.args, await glob(argv.args)];
}

if (include === undefined) {
return undefined;
}
Expand Down
11 changes: 5 additions & 6 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ export interface SucceededResult {
}

export const typeStat = async (
argv: TypeStatArgv,
configPath: string | undefined,
output: ProcessOutput,
): Promise<TypeStatResult> => {
const allPendingOptions = await tryLoadingPendingOptions(argv, output);
const allPendingOptions = await tryLoadingPendingOptions(configPath, output);
if (
allPendingOptions instanceof Error ||
typeof allPendingOptions === "string"
Expand Down Expand Up @@ -82,7 +82,7 @@ export const typeStat = async (
chalk.green(
` options ${pluralize(allPendingOptions.length, "object")} specified in `,
),
chalk.greenBright(argv.config),
chalk.greenBright(configPath),
chalk.green(` to modify your source code.`),
].join(""),
);
Expand All @@ -91,7 +91,6 @@ export const typeStat = async (
for (let i = 0; i < allPendingOptions.length; i += 1) {
// Collect all files to be run on this option iteration from the include glob(s)
const fileNames = await collectFileNames(
argv,
process.cwd(),
allPendingOptions[i].include,
);
Expand Down Expand Up @@ -146,11 +145,11 @@ export const typeStat = async (
};

const tryLoadingPendingOptions = async (
argv: TypeStatArgv,
configPath: string | undefined,
output: ProcessOutput,
): Promise<Error | PendingTypeStatOptions[] | string> => {
try {
return await loadPendingOptions(argv, output);
return await loadPendingOptions(configPath, output);
} catch (error) {
return error instanceof Error ? error : new Error(error as string);
}
Expand Down
2 changes: 0 additions & 2 deletions src/options/fillOutRawOptions.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { TypeStatArgv } from "../index.js";
import { ProcessOutput } from "../output/types.js";
import { collectOptionals } from "../shared/arrays.js";
import { ReactPropTypesHint, ReactPropTypesOptionality } from "./enums.js";
Expand All @@ -12,7 +11,6 @@ import { collectStrictNullChecks } from "./parsing/collectStrictNullChecks.js";
import { PendingTypeStatOptions, RawTypeStatOptions } from "./types.js";

export interface OptionsFromRawOptionsSettings {
argv: TypeStatArgv;
compilerOptions: Readonly<ParsedCompilerOptions>;
cwd: string;
output: ProcessOutput;
Expand Down
10 changes: 3 additions & 7 deletions src/options/loadPendingOptions.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import * as path from "node:path";

import { TypeStatArgv } from "../index.js";
import { ProcessOutput } from "../output/types.js";
import { normalizeAndSlashify } from "../shared/paths.js";
import { fillOutRawOptions } from "./fillOutRawOptions.js";
Expand All @@ -11,20 +10,18 @@ import { PendingTypeStatOptions, RawTypeStatOptions } from "./types.js";

/**
* Reads pre-file-rename TypeStat options using a config path.
* @param argv Root arguments passed to TypeStat.
* @param output Wraps process and logfile output.
* @returns Promise for filled-out TypeStat options, or a string complaint from failing to make them.
*/
export const loadPendingOptions = async (
argv: TypeStatArgv,
configPath: string | undefined,
output: ProcessOutput,
): Promise<PendingTypeStatOptions[] | string> => {
if (argv.config === undefined) {
if (configPath === undefined) {
return "-c/--config file must be provided.";
}

const cwd = process.cwd();
const foundRawOptions = findRawOptions(cwd, argv.config);
const foundRawOptions = findRawOptions(cwd, configPath);
if (typeof foundRawOptions === "string") {
return foundRawOptions;
}
Expand All @@ -40,7 +37,6 @@ export const loadPendingOptions = async (
const compilerOptions = await parseRawCompilerOptions(cwd, projectPath);

const filledOutOptions = fillOutRawOptions({
argv,
compilerOptions,
cwd,
output,
Expand Down
1 change: 0 additions & 1 deletion src/tests/testSetup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ export const runMutationTest = async (
};

const pendingOptions = fillOutRawOptions({
argv: { args: [] },
compilerOptions,
cwd: dirPath,
output,
Expand Down

0 comments on commit f21114e

Please sign in to comment.