Add restriction to `lint --fix`, `lint --review` and `ci` that errors with uncommitted changes. Add --allow-dirty flag to bypass.
MODIFY
packages/@romejs/cli/cli.ts
packages/@romejs/cli/cli.ts
@@ -109,6 +109,9 @@ export default async function cli() {
review: c.get('review').asBoolean(
DEFAULT_CLIENT_REQUEST_FLAGS.review,
),
+ allowDirty: c.get('allowDirty').asBoolean(
+ DEFAULT_CLIENT_REQUEST_FLAGS.allowDirty,
+ ),
watch: c.get('watch').asBoolean(DEFAULT_CLIENT_REQUEST_FLAGS.watch),
fieri: c.get('fieri').asBoolean(DEFAULT_CLIENT_REQUEST_FLAGS.fieri),
focus: c.get('focus').asString(DEFAULT_CLIENT_REQUEST_FLAGS.focus),
@@ -351,7 +354,9 @@ export default async function cli() {
}
case 'INVALID_REQUEST': {
+ if (res.showHelp) {
await p.showHelp();
+ }
process.exit(1);
break;
}

MODIFY
packages/@romejs/core/client/commands/lint.ts
packages/@romejs/core/client/commands/lint.ts
@@ -46,7 +46,7 @@ export default createLocalCommand<LintCommandFlags>(
{
category: commandCategories.CODE_QUALITY,
description: 'run lint against a set of files',
- allowRequestFlags: ['watch', 'review'],
+ allowRequestFlags: ['watch', 'review', 'allowDirty'],
usage: '',
examples: [],
@@ -220,6 +220,9 @@ export default createLocalCommand<LintCommandFlags>(
fix: true,
compilerOptions: lintOptionsPerFile,
},
+ requestFlags: {
+ allowDirty: true,
+ },
}, 'master');
return res2;
},

MODIFY
packages/@romejs/core/common/types/client.ts
packages/@romejs/core/common/types/client.ts
@@ -22,6 +22,7 @@ export const DEFAULT_CLIENT_FLAGS: ClientFlags = {
export const DEFAULT_CLIENT_REQUEST_FLAGS: ClientRequestFlags = {
collectMarkers: false,
timing: false,
+ allowDirty: false,
benchmark: false,
benchmarkIterations: 10,
@@ -37,6 +38,7 @@ export const DEFAULT_CLIENT_REQUEST_FLAGS: ClientRequestFlags = {
export type ClientRequestFlags = DiagnosticsPrinterFlags & {
watch: boolean;
review: boolean;
+ allowDirty: boolean;
// Debugging
timing: boolean;

MODIFY
packages/@romejs/core/master/Master.ts
packages/@romejs/core/master/Master.ts
@@ -17,6 +17,7 @@ import {
getDiagnosticsFromError,
deriveDiagnosticFromError,
DiagnosticsProcessor,
+ descriptions,
} from '@romejs/diagnostics';
import {MasterCommand, masterCommands} from './commands';
import {
@@ -25,7 +26,7 @@ import {
readDiagnosticsFileLocal,
printDiagnostics,
} from '@romejs/cli-diagnostics';
-import {consume, ConsumePath, Consumer} from '@romejs/consume';
+import {consume, ConsumePath} from '@romejs/consume';
import {Event, EventSubscription} from '@romejs/events';
import MasterRequest, {MasterRequestInvalid} from './MasterRequest';
import ProjectManager from './project/ProjectManager';
@@ -35,7 +36,6 @@ import FileAllocator from './fs/FileAllocator';
import Logger from '../common/utils/Logger';
import MemoryFileSystem from './fs/MemoryFileSystem';
import Cache from './Cache';
-
import {Reporter, ReporterStream} from '@romejs/cli-reporter';
import {Profiler} from '@romejs/v8';
import {
@@ -99,38 +99,44 @@ export type MasterMarker = MasterUnfinishedMarker & {
end: number;
};
-function validateRequestFlags(
+const disallowedFlagsWhenReviewing: Array<keyof ClientRequestFlags> = ['watch'];
+
+async function validateRequestFlags(
+ req: MasterRequest,
masterCommand: MasterCommand<Dict<unknown>>,
- flagsConsumer: Consumer,
) {
+ const {requestFlags} = req.query;
+
// Commands need to explicitly allow these flags
- validateAllowedRequestFlag('watch', masterCommand, flagsConsumer);
- validateAllowedRequestFlag('review', masterCommand, flagsConsumer);
+ validateAllowedRequestFlag(req, 'watch', masterCommand);
+ validateAllowedRequestFlag(req, 'review', masterCommand);
+ validateAllowedRequestFlag(req, 'allowDirty', masterCommand);
// Don't allow review in combination with other flags
- if (flagsConsumer.has('review')) {
- const disallowedFlagsWhenReviewing = ['watch'];
+ if (requestFlags.review) {
for (const key of disallowedFlagsWhenReviewing) {
- if (flagsConsumer.has(key)) {
- throw flagsConsumer.get(key).unexpected(
- `Flag <emphasis>${key}</emphasis> is not allowed with <emphasis>${key}</emphasis>`,
- );
+ if (requestFlags[key]) {
+ throw req.throwDiagnosticFlagError({
+ description: descriptions.FLAGS.DISALLOWED_REVIEW_FLAG(key),
+ target: {type: 'flag', key},
+ });
}
}
}
}
function validateAllowedRequestFlag(
+ req: MasterRequest,
flagKey: NonNullable<MasterCommand<Dict<unknown>>['allowRequestFlags']>[number],
masterCommand: MasterCommand<Dict<unknown>>,
- flagsConsumer: Consumer,
) {
const allowRequestFlags = masterCommand.allowRequestFlags || [];
- if (flagsConsumer.has(flagKey) && !allowRequestFlags.includes(flagKey)) {
- throw flagsConsumer.unexpected(
- `This command does not support the <emphasis>${flagKey}</emphasis> flag`,
- );
+ if (req.query.requestFlags && !allowRequestFlags.includes(flagKey)) {
+ throw req.throwDiagnosticFlagError({
+ description: descriptions.FLAGS.DISALLOWED_REQUEST_FLAG(flagKey),
+ target: {type: 'flag', key: flagKey},
+ });
}
}
@@ -774,16 +780,18 @@ export default class Master {
// A type-safe wrapper for retrieving command flags
// TODO perhaps present this as JSON or something if this isn't a request from the CLI?
- const commandFlagsConsumer = consume({
+ const flagsConsumer = consume({
filePath: createUnknownFilePath('argv'),
parent: undefined,
value: query.commandFlags,
+
onDefinition(def) {
// objectPath should only have a depth of 1
defaultCommandFlags[def.objectPath[0]] = def.default;
},
objectPath: [],
+
context: {
category: 'flags/invalid',
@@ -823,11 +831,11 @@ export default class Master {
this.warnedCacheClients.add(bridge);
}
- validateRequestFlags(masterCommand, commandFlagsConsumer);
+ await validateRequestFlags(req, masterCommand);
let commandFlags;
if (masterCommand.defineFlags !== undefined) {
- commandFlags = masterCommand.defineFlags(commandFlagsConsumer);
+ commandFlags = masterCommand.defineFlags(flagsConsumer);
}
req.setNormalizedCommandFlags({
@@ -880,6 +888,7 @@ export default class Master {
return {
type: 'INVALID_REQUEST',
diagnostics,
+ showHelp: err.showHelp,
};
} else {
return {
@@ -917,7 +926,11 @@ export default class Master {
// Only print when the bridge is alive and we aren't in review mode
// When we're in review mode we don't expect to show any diagnostics because they'll be intercepted in the client command
- if (req.bridge.alive && !req.query.requestFlags.review) {
+ // We will always print invalid request errors
+ const shouldPrint = req.bridge.alive && (rawErr instanceof
+ MasterRequestInvalid || !req.query.requestFlags.review);
+
+ if (shouldPrint) {
printer.print();
// Don't output the footer if this is a notifier for an invalid request as it will be followed by a help screen

MODIFY
packages/@romejs/core/master/MasterRequest.ts
packages/@romejs/core/master/MasterRequest.ts
@@ -17,9 +17,10 @@ import {
DiagnosticsError,
DiagnosticCategory,
Diagnostic,
- createBlessedDiagnosticMessage,
DiagnosticsProcessor,
Diagnostics,
+ DiagnosticDescription,
+ descriptions,
} from '@romejs/diagnostics';
import {
DiagnosticsPrinterFlags,
@@ -76,6 +77,7 @@ import {
DiagnosticsProcessorOptions,
} from '@romejs/diagnostics/DiagnosticsProcessor';
import {JSONObject} from '@romejs/codec-json';
+import {VCSClient} from '@romejs/vcs';
type MasterRequestOptions = {
master: Master;
@@ -116,7 +118,14 @@ export type MasterRequestGetFilesResult = {
paths: AbsoluteFilePathSet;
};
-export class MasterRequestInvalid extends DiagnosticsError {}
+export class MasterRequestInvalid extends DiagnosticsError {
+ constructor(message: string, diagnostics: Diagnostics, showHelp: boolean) {
+ super(message, diagnostics);
+ this.showHelp = showHelp;
+ }
+
+ showHelp: boolean;
+}
function hash(val: JSONObject): string {
return val === undefined || Object.keys(val).length === 0
@@ -201,6 +210,7 @@ export default class MasterRequest {
res = {
type: 'INVALID_REQUEST',
diagnostics: [],
+ showHelp: res.showHelp,
};
}
}
@@ -232,6 +242,38 @@ export default class MasterRequest {
);
}
+ async getVCSClient(): Promise<VCSClient> {
+ return this.master.projectManager.getVCSClient(
+ await this.assertClientCwdProject(),
+ );
+ }
+
+ async maybeGetVCSClient(): Promise<undefined | VCSClient> {
+ return this.master.projectManager.maybeGetVCSClient(
+ await this.assertClientCwdProject(),
+ );
+ }
+
+ async assertCleanVSC() {
+ if (this.query.requestFlags.allowDirty) {
+ return;
+ }
+
+ const vsc = await this.maybeGetVCSClient();
+ if (vsc === undefined) {
+ return;
+ }
+
+ const files = await vsc.getUncommittedFiles();
+ if (files.length > 0) {
+ this.throwDiagnosticFlagError({
+ description: descriptions.FLAGS.DIRTY_VSC(files),
+ target: {type: 'command'},
+ showHelp: false,
+ });
+ }
+ }
+
createDiagnosticsProcessor(
opts: DiagnosticsProcessorOptions = {},
): DiagnosticsProcessor {
@@ -299,41 +341,44 @@ export default class MasterRequest {
}
if (message !== undefined) {
- this.throwDiagnosticFlagError(excessive
- ? 'Too many arguments'
- : 'Missing arguments', {
+ this.throwDiagnosticFlagError({
+ target: {
type: 'arg-range',
from: min,
to: max,
- }, [
- {
- type: 'log',
- category: 'info',
- message,
},
- ]);
+ description: descriptions.FLAGS.INCORRECT_ARG_COUNT(excessive, message),
+ });
}
}
- throwDiagnosticFlagError(
- message: string,
- target: SerializeCLITarget = {type: 'none'},
- advice?: DiagnosticAdvice,
- ) {
+ throwDiagnosticFlagError({
+ description,
+ target = {type: 'none'},
+ showHelp = true,
+ }: {
+ description: RequiredProps<Partial<DiagnosticDescription>, 'message'>;
+ target?: SerializeCLITarget;
+ showHelp?: boolean;
+ }) {
const location = this.getDiagnosticPointerFromFlags(target);
+ let {category} = description;
+ if (category === undefined) {
+ category = target.type === 'arg' || target.type === 'arg-range'
+ ? 'args/invalid'
+ : 'flags/invalid';
+ }
+
const diag: Diagnostic = {
description: {
- message: createBlessedDiagnosticMessage(message),
- category: target.type === 'arg' || target.type === 'arg-range'
- ? 'args/invalid'
- : 'flags/invalid',
- advice,
+ ...description,
+ category,
},
location,
};
- throw new MasterRequestInvalid(message, [diag]);
+ throw new MasterRequestInvalid(description.message.value, [diag], showHelp);
}
getDiagnosticPointerForClientCwd(): DiagnosticLocation {
@@ -354,22 +399,31 @@ export default class MasterRequest {
};
}
- getDiagnosticPointerFromFlags(target: SerializeCLITarget): DiagnosticLocation {
- const {query} = this;
- return serializeCLIFlags({
- prefix: `rome ${query.commandName}`,
- flags: ({
- ...this.client.flags,
- ...query.requestFlags,
- ...this.normalizedCommandFlags.flags,
- } as Dict<unknown>),
- args: query.args,
- defaultFlags: {
+ getDefaultFlags(): Dict<unknown> {
+ return {
...DEFAULT_CLIENT_FLAGS,
...DEFAULT_CLIENT_REQUEST_FLAGS,
...this.normalizedCommandFlags.defaultFlags,
clientName: this.client.flags.clientName,
- },
+ };
+ }
+
+ getFlags(): Dict<unknown> {
+ return ({
+ ...this.client.flags,
+ ...this.query.requestFlags,
+ ...this.normalizedCommandFlags.flags,
+ } as Dict<unknown>);
+ }
+
+ getDiagnosticPointerFromFlags(target: SerializeCLITarget): DiagnosticLocation {
+ const {query} = this;
+ return serializeCLIFlags({
+ programName: 'rome',
+ commandName: query.commandName,
+ flags: this.getFlags(),
+ args: query.args,
+ defaultFlags: this.getDefaultFlags(),
incorrectCaseFlags: new Set(),
shorthandFlags: new Set(),
}, target);
@@ -688,10 +742,8 @@ export default class MasterRequest {
marker: `<filelink target="${path.join()}" />`,
},
description: {
+ ...descriptions.FLAGS.NO_FILES_FOUND(opts.noun),
category,
- message: createBlessedDiagnosticMessage(opts.noun === undefined
- ? 'No files found'
- : `No files to ${opts.noun} found`),
advice,
},
});

MODIFY
packages/@romejs/core/master/commands/ci.ts
packages/@romejs/core/master/commands/ci.ts
@@ -43,6 +43,7 @@ export default createMasterCommand({
description: 'run lint and tests',
usage: '',
examples: [],
+ allowRequestFlags: ['allowDirty'],
defineFlags(consumer: Consumer): Flags {
return {
@@ -53,6 +54,10 @@ export default createMasterCommand({
async callback(req: MasterRequest, flags: Flags): Promise<void> {
const {reporter} = req;
+ if (flags.fix) {
+ await req.assertCleanVSC();
+ }
+
reporter.heading('Running lint');
await runChildCommand(req, async () => {
await lint.callback(req, {

MODIFY
packages/@romejs/core/master/commands/lint.ts
packages/@romejs/core/master/commands/lint.ts
@@ -17,6 +17,10 @@ export default createMasterCommand<LintCommandFlags>({
async callback(req: MasterRequest, flags: LintCommandFlags): Promise<void> {
const {reporter} = req;
+ if (req.query.requestFlags.review || flags.fix) {
+ await req.assertCleanVSC();
+ }
+
const fixLocation = flags.fix === false
? undefined
: req.getDiagnosticPointerFromFlags({
@@ -30,9 +34,7 @@ export default createMasterCommand<LintCommandFlags>({
// No arguments expected when using this flag
req.expectArgumentLength(0);
- const client = await req.master.projectManager.getVCSClient(
- await req.assertClientCwdProject(),
- );
+ const client = await req.getVCSClient();
const target = flags.changed === '' ? client.trunkBranch : flags.changed;
args = await client.getModifiedFiles(target);

MODIFY
packages/@romejs/core/master/project/ProjectManager.ts
packages/@romejs/core/master/project/ProjectManager.ts
@@ -543,7 +543,7 @@ export default class ProjectManager {
}
async getVCSClient(project: ProjectDefinition): Promise<VCSClient> {
- const client = await getVCSClient(project.config.vcs.root);
+ const client = await this.maybeGetVCSClient(project);
if (client === undefined) {
const {
@@ -569,6 +569,12 @@ export default class ProjectManager {
}
}
+ async maybeGetVCSClient(
+ project: ProjectDefinition,
+ ): Promise<undefined | VCSClient> {
+ return await getVCSClient(project.config.vcs.root);
+ }
+
async addProject(
projectFolder: AbsoluteFilePath,
configPath: AbsoluteFilePath,

MODIFY
packages/@romejs/diagnostics/categories.ts
packages/@romejs/diagnostics/categories.ts
@@ -104,6 +104,7 @@ export type DiagnosticCategory =
| 'typeCheck/undeclaredVariable'
| 'typeCheck/unknownImport'
| 'typeCheck/unknownProperty'
+ | 'vsc/dirty'
| 'v8/syntaxError';
export type DiagnosticCategoryPrefix =

MODIFY
packages/@romejs/vcs/index.ts
packages/@romejs/vcs/index.ts
@@ -53,6 +53,21 @@ async function exec(command: string, args: Array<string>): Promise<string> {
);
}
+function extractFileList(out: string): Array<string> {
+ const lines = out.trim().split('\n');
+
+ const files: Array<string> = [];
+
+ for (const line of lines) {
+ const match = line.match(/^[AM]\s+(.*?)$/);
+ if (match != null) {
+ files.push(match[1]);
+ }
+ }
+
+ return files;
+}
+
export class VCSClient {
constructor(root: AbsoluteFilePath) {
this.root = root;
@@ -65,6 +80,10 @@ export class VCSClient {
getModifiedFiles(branch: string): Promise<Array<string>> {
throw new Error('unimplemented');
}
+
+ getUncommittedFiles(): Promise<Array<string>> {
+ throw new Error('unimplemented');
+ }
}
class GitVCSClient extends VCSClient {
@@ -73,20 +92,14 @@ class GitVCSClient extends VCSClient {
this.trunkBranch = 'master';
}
- async getModifiedFiles(branch: string): Promise<Array<string>> {
- const out = await exec('git', ['diff', '--name-status', branch]);
- const lines = out.trim().split('\n');
-
- const files: Array<string> = [];
-
- for (const line of lines) {
- const match = line.match(/^[AM]\s+(.*?)$/);
- if (match != null) {
- files.push(match[1]);
- }
+ async getUncommittedFiles(): Promise<Array<string>> {
+ const out = await exec('git', ['status', '--short']);
+ return extractFileList(out);
}
- return files;
+ async getModifiedFiles(branch: string): Promise<Array<string>> {
+ const out = await exec('git', ['diff', '--name-status', branch]);
+ return extractFileList(out);
}
}

Add restriction to `lint --fix`, `lint --review` and `ci` that errors with uncommitted changes. Add --allow-dirty flag to bypass.