From 7a86d97d4992e3d80d831eee3d77d4ca6b534fe6 Mon Sep 17 00:00:00 2001 From: rvanasa Date: Thu, 5 Feb 2026 15:26:12 -0500 Subject: [PATCH 1/6] Set up 'mops check' --- cli/CHANGELOG.md | 1 + cli/cli.ts | 22 ++++++++ cli/commands/check.ts | 69 ++++++++++++++++++++++++ cli/tests/__snapshots__/cli.test.ts.snap | 31 +++++++++++ cli/tests/check/error/Invalid.mo | 7 +++ cli/tests/check/error/mops.toml | 2 + cli/tests/check/success/Valid.mo | 5 ++ cli/tests/check/success/mops.toml | 2 + cli/tests/cli.test.ts | 12 +++++ 9 files changed, 151 insertions(+) create mode 100644 cli/commands/check.ts create mode 100644 cli/tests/check/error/Invalid.mo create mode 100644 cli/tests/check/error/mops.toml create mode 100644 cli/tests/check/success/Valid.mo create mode 100644 cli/tests/check/success/mops.toml diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md index a9261c18..213b5770 100644 --- a/cli/CHANGELOG.md +++ b/cli/CHANGELOG.md @@ -1,6 +1,7 @@ # Mops CLI Changelog ## Next +- Add `mops check` subcommand for type-checking Motoko files - Allow specifying toolchain file paths in `mops.toml` - Add `mops lint` subcommand and `lintoko` toolchain management diff --git a/cli/cli.ts b/cli/cli.ts index 3a3b1fb8..746bb01e 100755 --- a/cli/cli.ts +++ b/cli/cli.ts @@ -10,6 +10,7 @@ import { add } from "./commands/add.js"; import { bench } from "./commands/bench.js"; import { build, DEFAULT_BUILD_OUTPUT_DIR } from "./commands/build.js"; import { bump } from "./commands/bump.js"; +import { check } from "./commands/check.js"; import { checkCandid } from "./commands/check-candid.js"; import { docsCoverage } from "./commands/docs-coverage.js"; import { docs } from "./commands/docs.js"; @@ -289,6 +290,27 @@ program }); }); +// check +program + .command("check ") + .description("Check Motoko files for syntax errors and type issues") + .option("--verbose", "Verbose console output") + .allowUnknownOption(true) + .action(async (files, options, command) => { + checkConfigFile(true); + const extraArgsIndex = command.args.indexOf("--"); + await installAll({ + silent: true, + lock: "ignore", + installFromLockFile: true, + }); + await check(files, { + ...options, + extraArgs: + extraArgsIndex !== -1 ? command.args.slice(extraArgsIndex + 1) : [], + }); + }); + // check-candid program .command("check-candid ") diff --git a/cli/commands/check.ts b/cli/commands/check.ts new file mode 100644 index 00000000..0f7f7004 --- /dev/null +++ b/cli/commands/check.ts @@ -0,0 +1,69 @@ +import chalk from "chalk"; +import { execa } from "execa"; +import { cliError } from "../error.js"; +import { getMocPath } from "../helpers/get-moc-path.js"; +import { sourcesArgs } from "./sources.js"; + +export interface CheckOptions { + verbose: boolean; + extraArgs: string[]; +} + +export async function check( + files: string | string[], + options: Partial = {}, +): Promise { + const fileList = Array.isArray(files) ? files : [files]; + + if (fileList.length === 0) { + cliError("No Motoko files specified for checking"); + } + + let mocPath = getMocPath(); + let sources = await sourcesArgs(); + + for (const file of fileList) { + let args = [ + "--check", + file, + ...sources.flat(), + ...(options.extraArgs ?? []), + ]; + + try { + if (options.verbose) { + console.log(chalk.blue("check"), chalk.gray("Running moc:")); + console.log(chalk.gray(mocPath, JSON.stringify(args))); + } + + const result = await execa(mocPath, args, { + stdio: options.verbose ? "inherit" : "pipe", + reject: false, + }); + + if (result.exitCode !== 0) { + if (!options.verbose) { + if (result.stderr) { + console.error(chalk.red(result.stderr)); + } + if (result.stdout?.trim()) { + console.error(result.stdout); + } + } + cliError( + `✗ Check failed for file ${file} (exit code: ${result.exitCode})`, + ); + } + + if (options.verbose && result.stdout && result.stdout.trim()) { + console.log(result.stdout); + } + + console.log(chalk.green(`✓ ${file}`)); + } catch (err: any) { + cliError( + `Error while checking ${file}${err?.message ? `\n${err.message}` : ""}`, + ); + } + } +} diff --git a/cli/tests/__snapshots__/cli.test.ts.snap b/cli/tests/__snapshots__/cli.test.ts.snap index cbf49dc2..66aacd71 100644 --- a/cli/tests/__snapshots__/cli.test.ts.snap +++ b/cli/tests/__snapshots__/cli.test.ts.snap @@ -76,6 +76,37 @@ build canister bar } `; +exports[`mops check error 1`] = ` +{ + "exitCode": 1, + "stderr": "Invalid.mo:1.1-5.2: type error [M0096], expression of type + actor {hello : shared () -> async Text} +cannot produce expected type + () +Invalid.mo:7.1-7.21: type error [M0057], unbound variable thisshouldnotcompile +✗ Check failed for file Invalid.mo (exit code: 1)", + "stdout": "", +} +`; + +exports[`mops check success 1`] = ` +{ + "exitCode": 0, + "stderr": "", + "stdout": "✓ Valid.mo", +} +`; + +exports[`mops check success 2`] = ` +{ + "exitCode": 0, + "stderr": "", + "stdout": "check Running moc: +/Users/ryan/.motoko/bin/moc ["--check","Valid.mo","--package","core",".mops/core@2.0.0/src"] +✓ Valid.mo", +} +`; + exports[`mops check-candid 1`] = ` { "exitCode": 0, diff --git a/cli/tests/check/error/Invalid.mo b/cli/tests/check/error/Invalid.mo new file mode 100644 index 00000000..f18b2754 --- /dev/null +++ b/cli/tests/check/error/Invalid.mo @@ -0,0 +1,7 @@ +persistent actor { + public func hello() : async Text { + "Hello, World!"; + }; +}; + +thisshouldnotcompile diff --git a/cli/tests/check/error/mops.toml b/cli/tests/check/error/mops.toml new file mode 100644 index 00000000..89abf4c2 --- /dev/null +++ b/cli/tests/check/error/mops.toml @@ -0,0 +1,2 @@ +[dependencies] +core = "2.0.0" diff --git a/cli/tests/check/success/Valid.mo b/cli/tests/check/success/Valid.mo new file mode 100644 index 00000000..871f8fa8 --- /dev/null +++ b/cli/tests/check/success/Valid.mo @@ -0,0 +1,5 @@ +persistent actor { + public func hello() : async Text { + "Hello, World!"; + }; +}; diff --git a/cli/tests/check/success/mops.toml b/cli/tests/check/success/mops.toml new file mode 100644 index 00000000..89abf4c2 --- /dev/null +++ b/cli/tests/check/success/mops.toml @@ -0,0 +1,2 @@ +[dependencies] +core = "2.0.0" diff --git a/cli/tests/cli.test.ts b/cli/tests/cli.test.ts index b168d90e..a41962d3 100644 --- a/cli/tests/cli.test.ts +++ b/cli/tests/cli.test.ts @@ -76,6 +76,18 @@ describe("mops", () => { ).toMatch("Candid compatibility check failed for canister bar"); }); + test("check success", async () => { + const cwd = path.join(import.meta.dirname, "check/success"); + await cliSnapshot(["check", "Valid.mo"], { cwd }, 0); + await cliSnapshot(["check", "Valid.mo", "--verbose"], { cwd }, 0); + }); + + test("check error", async () => { + const cwd = path.join(import.meta.dirname, "check/error"); + await cliSnapshot(["check", "Invalid.mo"], { cwd }, 1); + await cliSnapshot(["check", "Valid.mo", "Invalid.mo"], { cwd }, 1); + }); + test("check-candid", async () => { const cwd = path.join(import.meta.dirname, "check-candid"); await cliSnapshot(["check-candid", "a.did", "a.did"], { cwd }, 0); From fd76d939817b7a6f5806901276da9b1b87592d20 Mon Sep 17 00:00:00 2001 From: rvanasa Date: Fri, 13 Feb 2026 13:39:33 -0500 Subject: [PATCH 2/6] Set up autofix logic --- cli/cli.ts | 1 + cli/commands/check.ts | 56 +++ cli/environments/nodejs/cli.ts | 8 +- cli/helpers/autofix-motoko.ts | 492 +++++++++++++++++++++++ cli/tests/__snapshots__/cli.test.ts.snap | 39 +- cli/tests/check/fix/M0223.mo | 7 + cli/tests/check/fix/M0236.mo | 11 + cli/tests/check/fix/M0237.mo | 7 + cli/tests/check/fix/Valid.mo | 7 + cli/tests/check/fix/mops.toml | 2 + cli/tests/cli.test.ts | 49 ++- 11 files changed, 658 insertions(+), 21 deletions(-) create mode 100644 cli/helpers/autofix-motoko.ts create mode 100644 cli/tests/check/fix/M0223.mo create mode 100644 cli/tests/check/fix/M0236.mo create mode 100644 cli/tests/check/fix/M0237.mo create mode 100644 cli/tests/check/fix/Valid.mo create mode 100644 cli/tests/check/fix/mops.toml diff --git a/cli/cli.ts b/cli/cli.ts index 746bb01e..bc527afb 100755 --- a/cli/cli.ts +++ b/cli/cli.ts @@ -295,6 +295,7 @@ program .command("check ") .description("Check Motoko files for syntax errors and type issues") .option("--verbose", "Verbose console output") + .addOption(new Option("--fix", "Apply autofixes")) .allowUnknownOption(true) .action(async (files, options, command) => { checkConfigFile(true); diff --git a/cli/commands/check.ts b/cli/commands/check.ts index 0f7f7004..004975cd 100644 --- a/cli/commands/check.ts +++ b/cli/commands/check.ts @@ -2,10 +2,12 @@ import chalk from "chalk"; import { execa } from "execa"; import { cliError } from "../error.js"; import { getMocPath } from "../helpers/get-moc-path.js"; +import { autofixMotoko } from "../helpers/autofix-motoko.js"; import { sourcesArgs } from "./sources.js"; export interface CheckOptions { verbose: boolean; + fix: boolean; extraArgs: string[]; } @@ -22,6 +24,60 @@ export async function check( let mocPath = getMocPath(); let sources = await sourcesArgs(); + // Helper function to compile and get errors + const compileErrors = async ( + filesToCheck: string[], + ): Promise => { + let allErrors = ""; + + for (const file of filesToCheck) { + let args = [ + "--check", + file, + ...sources.flat(), + ...(options.extraArgs ?? []), + ]; + + const result = await execa(mocPath, args, { + stdio: "pipe", + reject: false, + }); + + if (result.stderr) { + allErrors += result.stderr + "\n"; + } + if (result.stdout?.trim()) { + allErrors += result.stdout + "\n"; + } + } + + return allErrors.trim() ? allErrors.trim() : null; + }; + + // If fix flag is enabled, attempt to fix errors + if (options.fix) { + if (options.verbose) { + console.log(chalk.blue("check"), chalk.gray("Attempting to fix files")); + } + + const fixResult = await autofixMotoko(fileList, compileErrors); + if (fixResult) { + console.log( + chalk.green( + `✓ Fixed ${fixResult.fixedCount} file(s) with the following fixes:`, + ), + ); + for (const [code, count] of Object.entries(fixResult.fixedErrorCounts)) { + console.log(chalk.green(` ${code}: ${count} fix(es)`)); + } + } else { + if (options.verbose) { + console.log(chalk.yellow("No fixes were needed")); + } + } + } + + // Final check to verify all files pass for (const file of fileList) { let args = [ "--check", diff --git a/cli/environments/nodejs/cli.ts b/cli/environments/nodejs/cli.ts index ca96a813..a3bb07ab 100644 --- a/cli/environments/nodejs/cli.ts +++ b/cli/environments/nodejs/cli.ts @@ -1,6 +1,12 @@ -import * as wasm from "../../wasm/pkg/nodejs/wasm.js"; +import { createRequire } from "node:module"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; import { setWasmBindings } from "../../wasm.js"; +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const require = createRequire(import.meta.url); +const wasm = require(path.join(__dirname, "../../wasm/pkg/nodejs/wasm.js")); + setWasmBindings(wasm); export * from "../../cli.js"; diff --git a/cli/helpers/autofix-motoko.ts b/cli/helpers/autofix-motoko.ts new file mode 100644 index 00000000..b1096b5f --- /dev/null +++ b/cli/helpers/autofix-motoko.ts @@ -0,0 +1,492 @@ +import { readFileSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; + +interface Position { + line: number; + character: number; +} + +interface Range { + start: Position; + end: Position; +} + +interface TextEdit { + range: Range; + newText: string; +} + +interface Diagnostic { + file: string; + startLine: number; + startChar: number; + endLine: number; + endChar: number; + code: string; + message: string; +} + +interface CodeFix extends TextEdit { + code: string; +} + +class FileContent { + constructor(public readonly content: string) {} + + private lines: string[] | undefined; + private lineOffsets: number[] | undefined; + + getLines(): string[] { + this.lines ??= this.content.split("\n"); + return this.lines; + } + + getLineOffsets(): number[] { + if (!this.lineOffsets) { + let currentOffset = 0; + this.lineOffsets = this.getLines().map((l) => { + const off = currentOffset; + currentOffset += l.length + 1; + return off; + }); + } + return this.lineOffsets; + } + + getOffset(pos: Position): number { + const offsets = this.getLineOffsets(); + if (pos.line >= offsets.length) { + return this.content.length; + } + const offset = offsets[pos.line]; + return Math.min((offset ?? 0) + pos.character, this.content.length); + } + + textAt(range: Range): string { + const start = this.getOffset(range.start); + const end = this.getOffset(range.end); + return this.content.substring(start, end); + } + + lineAt(line: number): string { + const lines = this.getLines(); + return lines[line] ?? ""; + } +} + +function rangeFromDiagnostic(diag: Diagnostic): Range { + return { + start: { line: diag.startLine - 1, character: diag.startChar - 1 }, + end: { line: diag.endLine - 1, character: diag.endChar - 1 }, + }; +} + +export class MotokoFixer { + private readonly supportedCodes1 = ["M0223"]; + private readonly supportedCodes2 = ["M0236", "M0237"]; + private readonly initialErrorCounts = new Map(); + private readonly fixedErrorCounts = new Map(); + private readonly fileContentCache = new Map(); + + public fix( + files: Map, + errorOutput: string, + ): { + fixedFiles: Map; + fixedErrorCounts: Record; + } | null { + this.initialErrorCounts.clear(); + this.fixedErrorCounts.clear(); + this.fileContentCache.clear(); + + const allDiagnostics = this.parseDiagnostics(errorOutput); + if (allDiagnostics.length === 0) { + return null; + } + + for (const diag of allDiagnostics) { + this.initialErrorCounts.set( + diag.code, + (this.initialErrorCounts.get(diag.code) ?? 0) + 1, + ); + } + + const fixedFiles = new Map(); + const diagnosticsByFile = allDiagnostics.reduce((acc, diag) => { + const current = acc.get(diag.file) ?? []; + current.push(diag); + acc.set(diag.file, current); + return acc; + }, new Map()); + + for (const [file, diagnostics] of diagnosticsByFile) { + const fileKey = this.findFileKey(files, file); + if (!fileKey) { + continue; + } + + let fileContent = this.getFileContent(files, fileKey); + if (!fileContent) { + continue; + } + + // Phase 1: Apply simple fixes first to avoid overlapping fixes + const diagnostics1 = diagnostics.filter((diag) => + this.supportedCodes1.includes(diag.code), + ); + let fixes1 = this.generateFixes(diagnostics1, fileContent); + if (fixes1.length > 0) { + const result1 = this.applyFixes(fileContent, fixes1); + fixes1 = result1.applied; + fixedFiles.set(fileKey, result1.content); + fileContent = this.setFileContent(result1.content, fileKey); + } + + // Phase 2: Apply more complex fixes + let diagnostics2 = diagnostics.filter((diag) => + this.supportedCodes2.includes(diag.code), + ); + diagnostics2 = this.adjustDiagnosticPositions(diagnostics2, fixes1); + const fixes2 = this.generateFixes(diagnostics2, fileContent); + if (fixes2.length === 0) { + continue; + } + fixedFiles.set(fileKey, this.applyFixes(fileContent, fixes2).content); + } + + if (fixedFiles.size === 0) { + return null; + } + + return { + fixedFiles, + fixedErrorCounts: Object.fromEntries(this.fixedErrorCounts), + }; + } + + private findFileKey( + files: Map, + fileName: string, + ): string | null { + // Try exact match first + if (files.has(fileName)) { + return fileName; + } + + // Try matching the end of the file path + for (const key of files.keys()) { + if (key.endsWith(fileName) || key.endsWith(`/${fileName}`)) { + return key; + } + } + + return null; + } + + private generateFix( + diag: Diagnostic, + fileContent: FileContent, + ): TextEdit | null { + switch (diag.code) { + case "M0223": + return this.fixRedundantTypeInstantiation(diag); + case "M0236": + return this.fixDotNotationSuggestion(diag, fileContent); + case "M0237": + return this.fixRedundantImplicitArgument(diag, fileContent); + default: + return null; + } + } + + private generateFixes( + diagnostics: Diagnostic[], + fileContent: FileContent, + ): CodeFix[] { + const fixes: CodeFix[] = []; + for (const diag of diagnostics) { + const fix = this.generateFix(diag, fileContent); + if (fix) { + fixes.push({ ...fix, code: diag.code }); + } + } + // Sort by start position descending (bottom-up) + return fixes.sort( + (a, b) => + b.range.start.line - a.range.start.line || + b.range.start.character - a.range.start.character, + ); + } + + private fixRedundantTypeInstantiation(diag: Diagnostic): TextEdit { + return { range: rangeFromDiagnostic(diag), newText: "" }; + } + + private fixDotNotationSuggestion( + diag: Diagnostic, + fileContent: FileContent, + ): TextEdit | null { + const range = rangeFromDiagnostic(diag); + const originalText = fileContent.textAt(range); + + // Extract function name and first argument from the error message + // The message format is typically: "suggestion: use dot notation: obj.method(...)" + const match = originalText.match(/(\w+)\s*\(\s*(\w+)/); + if (!match) { + return null; + } + + const funcName = match[1]; + + // Simple replacement: convert func(obj, ...) to obj.func(...) + const newText = originalText.replace( + /(\w+)\s*\(\s*(\w+)/, + `$2.${funcName}(`, + ); + + return { range, newText }; + } + + private fixRedundantImplicitArgument( + diag: Diagnostic, + fileContent: FileContent, + ): TextEdit | null { + const range = rangeFromDiagnostic(diag); + + this.extendRangeWithComma(range, fileContent); + + const textAfter = fileContent + .lineAt(range.end.line) + .substring(range.end.character); + const textBefore = fileContent + .lineAt(range.start.line) + .substring(0, range.start.character); + + if (textBefore.trim() === "" && textAfter.trim() === "") { + range.start.character = 0; + range.end.line += 1; + range.end.character = 0; + } + + return { range, newText: "" }; + } + + private extendRangeWithComma(range: Range, fileContent: FileContent): void { + const textAfter = fileContent + .lineAt(range.end.line) + .substring(range.end.character); + const commaMatch = textAfter.match(/^\s*,\s*/); + if (commaMatch) { + range.end.character += commaMatch[0].length; + } + } + + public verifyFixes(newErrors: string): void { + const newCounts = new Map(); + for (const d of this.parseDiagnostics(newErrors)) { + newCounts.set(d.code, (newCounts.get(d.code) ?? 0) + 1); + } + + for (const [code, actualCount] of newCounts) { + if (code === "M0223") { + continue; // Cannot verify M0223 fixes + } + const initialCount = this.initialErrorCounts.get(code) ?? 0; + const fixedCount = this.fixedErrorCounts.get(code) ?? 0; + if (actualCount !== initialCount - fixedCount) { + console.warn( + `Warning: Incorrect error count for fix code ${code}: ` + + `${actualCount} !== ${initialCount} - ${fixedCount}`, + ); + } + } + } + + private parseDiagnostics(output: string): Diagnostic[] { + const regex = + /^([^:\n]+):(\d+)\.(\d+)-(\d+)\.(\d+): (?:type error|warning) \[(M\d+)\], (.+)$/gim; + return Array.from(output.matchAll(regex)).map((m) => { + const file = m[1] ?? ""; + const startLine = parseInt(m[2] ?? "0"); + const startChar = parseInt(m[3] ?? "0"); + const endLine = parseInt(m[4] ?? "0"); + const endChar = parseInt(m[5] ?? "0"); + const code = m[6] ?? ""; + const message = m[7] ?? ""; + return { + file, + startLine, + startChar, + endLine, + endChar, + code, + message, + }; + }); + } + + private applyFixes( + fileContent: FileContent, + fixes: CodeFix[], + ): { content: string; applied: CodeFix[] } { + let result = fileContent.content; + const applied: CodeFix[] = []; + let lastFixStartOffset = Infinity; + + for (const fix of fixes) { + const start = fileContent.getOffset(fix.range.start); + const end = fileContent.getOffset(fix.range.end); + + if (end > lastFixStartOffset) { + continue; + } + + result = result.slice(0, start) + fix.newText + result.slice(end); + lastFixStartOffset = start; + applied.push(fix); + + this.fixedErrorCounts.set( + fix.code, + (this.fixedErrorCounts.get(fix.code) ?? 0) + 1, + ); + } + return { content: result, applied }; + } + + private getFileContent( + files: Map, + fileKey: string, + ): FileContent | null { + let fileContent = this.fileContentCache.get(fileKey); + if (fileContent !== undefined) { + return fileContent; + } + + const content = files.get(fileKey); + if (typeof content !== "string") { + return null; + } + fileContent = new FileContent(content); + this.fileContentCache.set(fileKey, fileContent); + return fileContent; + } + + private setFileContent(content: string, fileKey: string): FileContent { + const fileContent = new FileContent(content); + this.fileContentCache.set(fileKey, fileContent); + return fileContent; + } + + private adjustDiagnosticPositions( + diagnostics: Diagnostic[], + fixes: CodeFix[], + ): Diagnostic[] { + const removedFromStart: number[] = diagnostics.map(() => 0); + const removedFromEnd: number[] = diagnostics.map(() => 0); + + for (const [index, diag] of diagnostics.entries()) { + const startLine = diag.startLine - 1; + const startChar = diag.startChar - 1; + const endLine = diag.endLine - 1; + const endChar = diag.endChar - 1; + + for (const fix of fixes) { + if (fix.newText !== "") { + continue; + } + if (fix.range.start.line !== fix.range.end.line) { + continue; + } + + const fixLine = fix.range.start.line; + const fixLen = fix.range.end.character - fix.range.start.character; + + if (fixLine === startLine && fixLen <= startChar) { + removedFromStart[index] = (removedFromStart[index] ?? 0) + fixLen; + } + + if (fixLine === endLine && fixLen <= endChar) { + removedFromEnd[index] = (removedFromEnd[index] ?? 0) + fixLen; + } + } + } + + return diagnostics.map((diag, index) => ({ + ...diag, + startChar: diag.startChar - (removedFromStart[index] ?? 0), + endChar: diag.endChar - (removedFromEnd[index] ?? 0), + })); + } +} + +export async function autofixMotoko( + files: string[], + compileErrors: (filePaths: string[]) => Promise, + baseDir: string = process.cwd(), +): Promise<{ + fixedCount: number; + fixedErrorCounts: Record; +} | null> { + // Load file contents + const fileContents = new Map(); + const filePaths = new Map(); // maps relative path to absolute path + + for (const file of files) { + const absolutePath = file.startsWith("/") ? file : join(baseDir, file); + const content = readFileSync(absolutePath, "utf-8"); + fileContents.set(file, content); + filePaths.set(file, absolutePath); + } + + let currentFiles = fileContents; + let currentErrors = await compileErrors(files); + let totalFixedCount = 0; + const allFixedErrorCounts: Record = {}; + + const fixer = new MotokoFixer(); + const maxIterations = 10; + + for (let i = 0; i < maxIterations && currentErrors !== null; i++) { + const fixResult = fixer.fix(currentFiles, currentErrors); + if (!fixResult) { + break; + } + + // Merge fixed files + for (const [file, content] of fixResult.fixedFiles) { + currentFiles.set(file, content); + } + + totalFixedCount += fixResult.fixedFiles.size; + + // Accumulate fixed error counts + for (const [code, count] of Object.entries(fixResult.fixedErrorCounts)) { + allFixedErrorCounts[code] = (allFixedErrorCounts[code] ?? 0) + count; + } + + currentErrors = await compileErrors(files); + if (currentErrors !== null) { + fixer.verifyFixes(currentErrors); + } + } + + if (totalFixedCount === 0) { + return null; + } + + // Write fixed files back to disk + for (const file of files) { + const absolutePath = filePaths.get(file); + if (absolutePath && currentFiles.has(file)) { + const fixedContent = currentFiles.get(file); + if (fixedContent) { + writeFileSync(absolutePath, fixedContent, "utf-8"); + } + } + } + + return { + fixedCount: totalFixedCount, + fixedErrorCounts: allFixedErrorCounts, + }; +} diff --git a/cli/tests/__snapshots__/cli.test.ts.snap b/cli/tests/__snapshots__/cli.test.ts.snap index 66aacd71..ef973b4a 100644 --- a/cli/tests/__snapshots__/cli.test.ts.snap +++ b/cli/tests/__snapshots__/cli.test.ts.snap @@ -89,6 +89,15 @@ Invalid.mo:7.1-7.21: type error [M0057], unbound variable thisshouldnotcompile } `; +exports[`mops check error 2`] = ` +{ + "exitCode": 1, + "stderr": "Valid.mo: No such file or directory +✗ Check failed for file Valid.mo (exit code: 1)", + "stdout": "", +} +`; + exports[`mops check success 1`] = ` { "exitCode": 0, @@ -102,7 +111,7 @@ exports[`mops check success 2`] = ` "exitCode": 0, "stderr": "", "stdout": "check Running moc: -/Users/ryan/.motoko/bin/moc ["--check","Valid.mo","--package","core",".mops/core@2.0.0/src"] +moc-wrapper ["--check","Valid.mo","--package","core",".mops/core@2.0.0/src"] ✓ Valid.mo", } `; @@ -198,12 +207,12 @@ Lint failed with exit code 1", "stdout": "lint Running lintoko: /lintoko/0.7.0/lintoko ["--verbose","--rules","lints","/lint/src/Valid.mo","/lint/src/NoBoolSwitch.mo"] -DEBUG file input: /lint/src/Valid.mo -DEBUG file input: /lint/src/NoBoolSwitch.mo -DEBUG Loading rules from: lints -DEBUG Parsing extra rule at: lints/no-bool-switch.toml -DEBUG Linting file: /lint/src/NoBoolSwitch.mo -DEBUG Linting file: /lint/src/Valid.mo", +DEBUG file input: /lint/src/Valid.mo +DEBUG file input: /lint/src/NoBoolSwitch.mo +DEBUG Loading rules from: lints +DEBUG Parsing extra rule at: lints/no-bool-switch.toml +DEBUG Linting file: /lint/src/NoBoolSwitch.mo +DEBUG Linting file: /lint/src/Valid.mo", } `; @@ -214,10 +223,10 @@ exports[`mops lint 2`] = ` "stdout": "lint Running lintoko: /lintoko/0.7.0/lintoko ["--verbose","--rules","lints","/lint/src/Valid.mo"] -DEBUG file input: /lint/src/Valid.mo -DEBUG Loading rules from: lints -DEBUG Parsing extra rule at: lints/no-bool-switch.toml -DEBUG Linting file: /lint/src/Valid.mo +DEBUG file input: /lint/src/Valid.mo +DEBUG Loading rules from: lints +DEBUG Parsing extra rule at: lints/no-bool-switch.toml +DEBUG Linting file: /lint/src/Valid.mo ✓ Lint succeeded", } `; @@ -241,10 +250,10 @@ Lint failed with exit code 1", "stdout": "lint Running lintoko: /lintoko/0.7.0/lintoko ["--verbose","--rules","lints","/lint/src/NoBoolSwitch.mo"] -DEBUG file input: /lint/src/NoBoolSwitch.mo -DEBUG Loading rules from: lints -DEBUG Parsing extra rule at: lints/no-bool-switch.toml -DEBUG Linting file: /lint/src/NoBoolSwitch.mo", +DEBUG file input: /lint/src/NoBoolSwitch.mo +DEBUG Loading rules from: lints +DEBUG Parsing extra rule at: lints/no-bool-switch.toml +DEBUG Linting file: /lint/src/NoBoolSwitch.mo", } `; diff --git a/cli/tests/check/fix/M0223.mo b/cli/tests/check/fix/M0223.mo new file mode 100644 index 00000000..1d1fdde6 --- /dev/null +++ b/cli/tests/check/fix/M0223.mo @@ -0,0 +1,7 @@ +// M0223: Redundant type instantiation +// The type annotation is not needed when it can be inferred +persistent actor { + public func testM0223() : async Nat { + let x : Nat = 42; + }; +}; diff --git a/cli/tests/check/fix/M0236.mo b/cli/tests/check/fix/M0236.mo new file mode 100644 index 00000000..e3007900 --- /dev/null +++ b/cli/tests/check/fix/M0236.mo @@ -0,0 +1,11 @@ +// M0236: Suggested to use dot notation +// Function calls can be rewritten using dot notation +import Array "mo:core/Array"; + +persistent actor { + public func testM0236() : async Nat { + let arr = [1, 2, 3]; + let len = Array.size(arr); + len + }; +}; diff --git a/cli/tests/check/fix/M0237.mo b/cli/tests/check/fix/M0237.mo new file mode 100644 index 00000000..b04bc62a --- /dev/null +++ b/cli/tests/check/fix/M0237.mo @@ -0,0 +1,7 @@ +// M0237: Redundant explicit implicit argument +// Some arguments can be inferred and don't need to be specified +persistent actor { + public func testM0237() : async () { + let _x : ?Text = null; + }; +}; diff --git a/cli/tests/check/fix/Valid.mo b/cli/tests/check/fix/Valid.mo new file mode 100644 index 00000000..607a6686 --- /dev/null +++ b/cli/tests/check/fix/Valid.mo @@ -0,0 +1,7 @@ +// File with M0237 redundant implicit argument that can be auto-fixed +persistent actor { + public func example() : async () { + let _x : ?Text = null; + (); + }; +}; diff --git a/cli/tests/check/fix/mops.toml b/cli/tests/check/fix/mops.toml new file mode 100644 index 00000000..89abf4c2 --- /dev/null +++ b/cli/tests/check/fix/mops.toml @@ -0,0 +1,2 @@ +[dependencies] +core = "2.0.0" diff --git a/cli/tests/cli.test.ts b/cli/tests/cli.test.ts index a41962d3..64e714cc 100644 --- a/cli/tests/cli.test.ts +++ b/cli/tests/cli.test.ts @@ -9,18 +9,27 @@ interface CliOptions { const cli = async (args: string[], { cwd }: CliOptions = {}) => { return await execa("npm", ["run", "--silent", "mops", "--", ...args], { - env: { MOPS_CWD: cwd }, + env: { ...process.env, ...(cwd != null && { MOPS_CWD: cwd }) }, + ...(cwd != null && { cwd }), stdio: "pipe", reject: false, }); }; +// Strip ANSI escape codes for portable snapshots (avoid control char in regex literal) +const stripAnsi = (s: string) => + s.replace(new RegExp(`\u001b\\[[0-9;]*m`, "g"), ""); + const normalizePaths = (text: string): string => { // Replace absolute paths with placeholders for CI - return text - .replaceAll(dirname(fileURLToPath(import.meta.url)), "") - .replace(/\/[^\s"]+\/\.cache\/mops/g, "") - .replace(/\/[^\s"]+\/Library\/Caches\/mops/g, ""); + return stripAnsi( + text + .replaceAll(dirname(fileURLToPath(import.meta.url)), "") + .replace(/\/[^\s"]+\/\.cache\/mops/g, "") + .replace(/\/[^\s"]+\/Library\/Caches\/mops/g, "") + .replace(/\/[^\s"[\]]+\/moc(?:-wrapper)?(?=\s|$)/g, "moc-wrapper") + .replace(/\/[^\s"[\]]+\.motoko\/bin\/moc/g, "moc-wrapper"), + ); }; const cliSnapshot = async ( @@ -88,6 +97,36 @@ describe("mops", () => { await cliSnapshot(["check", "Valid.mo", "Invalid.mo"], { cwd }, 1); }); + test("check --fix M0223", async () => { + const cwd = path.join(import.meta.dirname, "check/fix"); + const result = await cli(["check", "M0223.mo", "--fix"], { cwd }); + expect(result.exitCode).toBe(0); + expect(result.stdout).toBeTruthy(); + }); + + test("check --fix M0236", async () => { + const cwd = path.join(import.meta.dirname, "check/fix"); + const result = await cli(["check", "M0236.mo", "--fix"], { cwd }); + expect(result.exitCode).toBe(0); + expect(result.stdout).toBeTruthy(); + }); + + test("check --fix M0237", async () => { + const cwd = path.join(import.meta.dirname, "check/fix"); + const result = await cli(["check", "M0237.mo", "--fix"], { cwd }); + expect(result.exitCode).toBe(0); + expect(result.stdout).toBeTruthy(); + }); + + test("check --fix verbose", async () => { + const cwd = path.join(import.meta.dirname, "check/fix"); + const result = await cli(["check", "Valid.mo", "--fix", "--verbose"], { + cwd, + }); + expect(result.exitCode).toBe(0); + expect(result.stdout).toBeTruthy(); + }); + test("check-candid", async () => { const cwd = path.join(import.meta.dirname, "check-candid"); await cliSnapshot(["check-candid", "a.did", "a.did"], { cwd }, 0); From e326569094452a6c7008f774fe09f8b774ff620f Mon Sep 17 00:00:00 2001 From: rvanasa Date: Tue, 17 Feb 2026 08:54:42 -0500 Subject: [PATCH 3/6] Format --- cli/tests/check/error/Invalid.mo | 2 +- cli/tests/check/fix/M0223.mo | 2 +- cli/tests/check/fix/M0236.mo | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cli/tests/check/error/Invalid.mo b/cli/tests/check/error/Invalid.mo index f18b2754..23f559d2 100644 --- a/cli/tests/check/error/Invalid.mo +++ b/cli/tests/check/error/Invalid.mo @@ -4,4 +4,4 @@ persistent actor { }; }; -thisshouldnotcompile +thisshouldnotcompile; diff --git a/cli/tests/check/fix/M0223.mo b/cli/tests/check/fix/M0223.mo index 1d1fdde6..31fca87e 100644 --- a/cli/tests/check/fix/M0223.mo +++ b/cli/tests/check/fix/M0223.mo @@ -1,7 +1,7 @@ // M0223: Redundant type instantiation // The type annotation is not needed when it can be inferred persistent actor { - public func testM0223() : async Nat { + public func testM0223() : async Nat { let x : Nat = 42; }; }; diff --git a/cli/tests/check/fix/M0236.mo b/cli/tests/check/fix/M0236.mo index e3007900..671d3227 100644 --- a/cli/tests/check/fix/M0236.mo +++ b/cli/tests/check/fix/M0236.mo @@ -6,6 +6,6 @@ persistent actor { public func testM0236() : async Nat { let arr = [1, 2, 3]; let len = Array.size(arr); - len + len; }; }; From a9688128fda10d5da8c8eb3c7bee5140baf3ce77 Mon Sep 17 00:00:00 2001 From: rvanasa Date: Tue, 17 Feb 2026 11:47:32 -0500 Subject: [PATCH 4/6] Improve tests and add '--warnings' flag --- cli/cli.ts | 1 + cli/commands/check.ts | 3 + cli/tests/__snapshots__/cli.test.ts.snap | 55 ++++++++++++---- .../check/error/{Invalid.mo => Error.mo} | 0 cli/tests/check/fix/M0223.mo | 2 +- cli/tests/check/fix/{Valid.mo => Ok.mo} | 0 cli/tests/check/fix/mops.toml | 3 + cli/tests/check/fix/run/.gitignore | 1 + cli/tests/check/success/{Valid.mo => Ok.mo} | 0 cli/tests/check/success/Warning.mo | 5 ++ cli/tests/cli.test.ts | 66 ++++++++++++++----- cli/tests/lint/src/{Valid.mo => Ok.mo} | 0 12 files changed, 104 insertions(+), 32 deletions(-) rename cli/tests/check/error/{Invalid.mo => Error.mo} (100%) rename cli/tests/check/fix/{Valid.mo => Ok.mo} (100%) create mode 100644 cli/tests/check/fix/run/.gitignore rename cli/tests/check/success/{Valid.mo => Ok.mo} (100%) create mode 100644 cli/tests/check/success/Warning.mo rename cli/tests/lint/src/{Valid.mo => Ok.mo} (100%) diff --git a/cli/cli.ts b/cli/cli.ts index c5a27e97..32ca7cfc 100755 --- a/cli/cli.ts +++ b/cli/cli.ts @@ -296,6 +296,7 @@ program .description("Check Motoko files for syntax errors and type issues") .option("--verbose", "Verbose console output") .addOption(new Option("--fix", "Apply autofixes")) + .addOption(new Option("--warnings", "Treat warnings as errors")) .allowUnknownOption(true) .action(async (files, options, command) => { checkConfigFile(true); diff --git a/cli/commands/check.ts b/cli/commands/check.ts index 004975cd..d1e8fdd4 100644 --- a/cli/commands/check.ts +++ b/cli/commands/check.ts @@ -8,6 +8,7 @@ import { sourcesArgs } from "./sources.js"; export interface CheckOptions { verbose: boolean; fix: boolean; + warnings: boolean; extraArgs: string[]; } @@ -35,6 +36,7 @@ export async function check( "--check", file, ...sources.flat(), + ...(options.warnings ? ["-Werror"] : []), ...(options.extraArgs ?? []), ]; @@ -83,6 +85,7 @@ export async function check( "--check", file, ...sources.flat(), + ...(options.warnings ? ["-Werror"] : []), ...(options.extraArgs ?? []), ]; diff --git a/cli/tests/__snapshots__/cli.test.ts.snap b/cli/tests/__snapshots__/cli.test.ts.snap index ef973b4a..2d68b232 100644 --- a/cli/tests/__snapshots__/cli.test.ts.snap +++ b/cli/tests/__snapshots__/cli.test.ts.snap @@ -79,12 +79,12 @@ build canister bar exports[`mops check error 1`] = ` { "exitCode": 1, - "stderr": "Invalid.mo:1.1-5.2: type error [M0096], expression of type + "stderr": "Error.mo:1.1-5.2: type error [M0096], expression of type actor {hello : shared () -> async Text} cannot produce expected type () -Invalid.mo:7.1-7.21: type error [M0057], unbound variable thisshouldnotcompile -✗ Check failed for file Invalid.mo (exit code: 1)", +Error.mo:7.1-7.21: type error [M0057], unbound variable thisshouldnotcompile +✗ Check failed for file Error.mo (exit code: 1)", "stdout": "", } `; @@ -92,8 +92,8 @@ Invalid.mo:7.1-7.21: type error [M0057], unbound variable thisshouldnotcompile exports[`mops check error 2`] = ` { "exitCode": 1, - "stderr": "Valid.mo: No such file or directory -✗ Check failed for file Valid.mo (exit code: 1)", + "stderr": "Ok.mo: No such file or directory +✗ Check failed for file Ok.mo (exit code: 1)", "stdout": "", } `; @@ -102,7 +102,7 @@ exports[`mops check success 1`] = ` { "exitCode": 0, "stderr": "", - "stdout": "✓ Valid.mo", + "stdout": "✓ Ok.mo", } `; @@ -111,8 +111,35 @@ exports[`mops check success 2`] = ` "exitCode": 0, "stderr": "", "stdout": "check Running moc: -moc-wrapper ["--check","Valid.mo","--package","core",".mops/core@2.0.0/src"] -✓ Valid.mo", +moc-wrapper ["--check","Ok.mo","--package","core",".mops/core@2.0.0/src"] +✓ Ok.mo", +} +`; + +exports[`mops check warning 1`] = ` +{ + "exitCode": 0, + "stderr": "", + "stdout": "✓ M0223.mo", +} +`; + +exports[`mops check warning verbose 1`] = ` +{ + "exitCode": 0, + "stderr": "Warning.mo:3.9-3.15: warning [M0194], unused identifier unused (delete or rename to wildcard \`_\` or \`_unused\`)", + "stdout": "check Running moc: +moc-wrapper ["--check","Warning.mo","--package","core",".mops/core@2.0.0/src"] +✓ Warning.mo", +} +`; + +exports[`mops check warning with --warnings flag 1`] = ` +{ + "exitCode": 1, + "stderr": "Warning.mo:3.9-3.15: warning [M0194], unused identifier unused (delete or rename to wildcard \`_\` or \`_unused\`) +✗ Check failed for file Warning.mo (exit code: 1)", + "stdout": "", } `; @@ -206,13 +233,13 @@ Error: Found 1 errors Lint failed with exit code 1", "stdout": "lint Running lintoko: /lintoko/0.7.0/lintoko -["--verbose","--rules","lints","/lint/src/Valid.mo","/lint/src/NoBoolSwitch.mo"] -DEBUG file input: /lint/src/Valid.mo +["--verbose","--rules","lints","/lint/src/Ok.mo","/lint/src/NoBoolSwitch.mo"] +DEBUG file input: /lint/src/Ok.mo DEBUG file input: /lint/src/NoBoolSwitch.mo DEBUG Loading rules from: lints DEBUG Parsing extra rule at: lints/no-bool-switch.toml DEBUG Linting file: /lint/src/NoBoolSwitch.mo -DEBUG Linting file: /lint/src/Valid.mo", +DEBUG Linting file: /lint/src/Ok.mo", } `; @@ -222,11 +249,11 @@ exports[`mops lint 2`] = ` "stderr": "", "stdout": "lint Running lintoko: /lintoko/0.7.0/lintoko -["--verbose","--rules","lints","/lint/src/Valid.mo"] -DEBUG file input: /lint/src/Valid.mo +["--verbose","--rules","lints","/lint/src/Ok.mo"] +DEBUG file input: /lint/src/Ok.mo DEBUG Loading rules from: lints DEBUG Parsing extra rule at: lints/no-bool-switch.toml -DEBUG Linting file: /lint/src/Valid.mo +DEBUG Linting file: /lint/src/Ok.mo ✓ Lint succeeded", } `; diff --git a/cli/tests/check/error/Invalid.mo b/cli/tests/check/error/Error.mo similarity index 100% rename from cli/tests/check/error/Invalid.mo rename to cli/tests/check/error/Error.mo diff --git a/cli/tests/check/fix/M0223.mo b/cli/tests/check/fix/M0223.mo index 31fca87e..d3f52c61 100644 --- a/cli/tests/check/fix/M0223.mo +++ b/cli/tests/check/fix/M0223.mo @@ -2,6 +2,6 @@ // The type annotation is not needed when it can be inferred persistent actor { public func testM0223() : async Nat { - let x : Nat = 42; + let _x : Nat = 42; }; }; diff --git a/cli/tests/check/fix/Valid.mo b/cli/tests/check/fix/Ok.mo similarity index 100% rename from cli/tests/check/fix/Valid.mo rename to cli/tests/check/fix/Ok.mo diff --git a/cli/tests/check/fix/mops.toml b/cli/tests/check/fix/mops.toml index 89abf4c2..3df17069 100644 --- a/cli/tests/check/fix/mops.toml +++ b/cli/tests/check/fix/mops.toml @@ -1,2 +1,5 @@ [dependencies] core = "2.0.0" + +[toolchain] +moc = "1.2.0" diff --git a/cli/tests/check/fix/run/.gitignore b/cli/tests/check/fix/run/.gitignore new file mode 100644 index 00000000..d0f6ec94 --- /dev/null +++ b/cli/tests/check/fix/run/.gitignore @@ -0,0 +1 @@ +**/*.mo diff --git a/cli/tests/check/success/Valid.mo b/cli/tests/check/success/Ok.mo similarity index 100% rename from cli/tests/check/success/Valid.mo rename to cli/tests/check/success/Ok.mo diff --git a/cli/tests/check/success/Warning.mo b/cli/tests/check/success/Warning.mo new file mode 100644 index 00000000..9cc5400f --- /dev/null +++ b/cli/tests/check/success/Warning.mo @@ -0,0 +1,5 @@ +persistent actor { + public func example() : async () { + let unused = 123; + }; +}; diff --git a/cli/tests/cli.test.ts b/cli/tests/cli.test.ts index 64e714cc..33afce31 100644 --- a/cli/tests/cli.test.ts +++ b/cli/tests/cli.test.ts @@ -1,4 +1,5 @@ import { describe, expect, test } from "@jest/globals"; +import { cpSync, mkdirSync, readFileSync } from "node:fs"; import { execa } from "execa"; import path, { dirname } from "path"; import { fileURLToPath } from "url"; @@ -87,40 +88,71 @@ describe("mops", () => { test("check success", async () => { const cwd = path.join(import.meta.dirname, "check/success"); - await cliSnapshot(["check", "Valid.mo"], { cwd }, 0); - await cliSnapshot(["check", "Valid.mo", "--verbose"], { cwd }, 0); + await cliSnapshot(["check", "Ok.mo"], { cwd }, 0); + await cliSnapshot(["check", "Ok.mo", "--verbose"], { cwd }, 0); }); test("check error", async () => { const cwd = path.join(import.meta.dirname, "check/error"); - await cliSnapshot(["check", "Invalid.mo"], { cwd }, 1); - await cliSnapshot(["check", "Valid.mo", "Invalid.mo"], { cwd }, 1); + await cliSnapshot(["check", "Error.mo"], { cwd }, 1); + await cliSnapshot(["check", "Ok.mo", "Error.mo"], { cwd }, 1); }); - test("check --fix M0223", async () => { + test("check warning", async () => { const cwd = path.join(import.meta.dirname, "check/fix"); - const result = await cli(["check", "M0223.mo", "--fix"], { cwd }); + await cliSnapshot(["check", "M0223.mo"], { cwd }, 0); + }); + + test("check warning verbose", async () => { + const cwd = path.join(import.meta.dirname, "check/success"); + const result = await cliSnapshot( + ["check", "Warning.mo", "--verbose"], + { cwd }, + 0, + ); + // Verify the warning is shown in stderr without --warnings flag + expect(result.stderr).toMatch(/warning \[M0194\]/); + expect(result.stderr).toMatch(/unused identifier/); + }); + + test("check warning with --warnings flag", async () => { + const cwd = path.join(import.meta.dirname, "check/success"); + // With --warnings flag, warnings should cause check to fail + await cliSnapshot(["check", "Warning.mo", "--warnings"], { cwd }, 1); + }); + + const fixDir = path.join(import.meta.dirname, "check/fix"); + const runDir = path.join(fixDir, "run"); + + async function checkFix(file: string, original: string, expected: string) { + mkdirSync(runDir, { recursive: true }); + cpSync(path.join(fixDir, file), path.join(runDir, file)); + const before = readFileSync(path.join(runDir, file), "utf-8"); + expect(before).toContain(original); + const result = await cli(["check", `run/${file}`, "--fix"], { + cwd: fixDir, + }); expect(result.exitCode).toBe(0); - expect(result.stdout).toBeTruthy(); + const after = readFileSync(path.join(runDir, file), "utf-8"); + expect(after).toContain(expected); + expect(after).not.toContain(original); + } + + test("check --fix M0223", async () => { + await checkFix("M0223.mo", "let _x : Nat = 42", "let _x = 42"); }); test("check --fix M0236", async () => { - const cwd = path.join(import.meta.dirname, "check/fix"); - const result = await cli(["check", "M0236.mo", "--fix"], { cwd }); - expect(result.exitCode).toBe(0); - expect(result.stdout).toBeTruthy(); + await checkFix("M0236.mo", "Array.size(arr)", "arr.size()"); }); test("check --fix M0237", async () => { - const cwd = path.join(import.meta.dirname, "check/fix"); - const result = await cli(["check", "M0237.mo", "--fix"], { cwd }); - expect(result.exitCode).toBe(0); - expect(result.stdout).toBeTruthy(); + await checkFix("M0237.mo", "let _x : ?Text = null", "let _x = null"); }); test("check --fix verbose", async () => { const cwd = path.join(import.meta.dirname, "check/fix"); - const result = await cli(["check", "Valid.mo", "--fix", "--verbose"], { + const result = await cli(["check", "Ok.mo", "--fix", "--verbose"], { cwd, }); expect(result.exitCode).toBe(0); @@ -143,7 +175,7 @@ describe("mops", () => { test("lint", async () => { const cwd = path.join(import.meta.dirname, "lint"); await cliSnapshot(["lint", "--verbose"], { cwd }, 1); - await cliSnapshot(["lint", "Valid", "--verbose"], { cwd }, 0); + await cliSnapshot(["lint", "Ok", "--verbose"], { cwd }, 0); await cliSnapshot(["lint", "NoBoolSwitch", "--verbose"], { cwd }, 1); await cliSnapshot(["lint", "DoesNotExist"], { cwd }, 1); }); diff --git a/cli/tests/lint/src/Valid.mo b/cli/tests/lint/src/Ok.mo similarity index 100% rename from cli/tests/lint/src/Valid.mo rename to cli/tests/lint/src/Ok.mo From 409b8968080fde978a59afd49b26b9dbfd7dbafb Mon Sep 17 00:00:00 2001 From: rvanasa Date: Wed, 18 Feb 2026 11:48:18 -0500 Subject: [PATCH 5/6] Refactor tests --- cli/cli.ts | 42 ++- cli/commands/check.ts | 3 - cli/tests/__snapshots__/build.test.ts.snap | 77 +++++ .../__snapshots__/check-candid.test.ts.snap | 73 +++++ cli/tests/__snapshots__/check.test.ts.snap | 68 ++++ cli/tests/__snapshots__/cli.test.ts.snap | 293 ------------------ cli/tests/__snapshots__/lint.test.ts.snap | 78 +++++ cli/tests/build.test.ts | 24 ++ cli/tests/check-candid.test.ts | 22 ++ cli/tests/check.test.ts | 92 ++++++ cli/tests/check/fix/M0223.mo | 7 +- cli/tests/check/fix/M0236.mo | 10 +- cli/tests/check/fix/M0237.mo | 6 +- cli/tests/cli.test.ts | 185 +---------- cli/tests/helpers.ts | 58 ++++ cli/tests/lint.test.ts | 17 + cli/tests/toolchain.test.ts | 12 + 17 files changed, 567 insertions(+), 500 deletions(-) create mode 100644 cli/tests/__snapshots__/build.test.ts.snap create mode 100644 cli/tests/__snapshots__/check-candid.test.ts.snap create mode 100644 cli/tests/__snapshots__/check.test.ts.snap delete mode 100644 cli/tests/__snapshots__/cli.test.ts.snap create mode 100644 cli/tests/__snapshots__/lint.test.ts.snap create mode 100644 cli/tests/build.test.ts create mode 100644 cli/tests/check-candid.test.ts create mode 100644 cli/tests/check.test.ts create mode 100644 cli/tests/helpers.ts create mode 100644 cli/tests/lint.test.ts create mode 100644 cli/tests/toolchain.test.ts diff --git a/cli/cli.ts b/cli/cli.ts index 32ca7cfc..47224255 100755 --- a/cli/cli.ts +++ b/cli/cli.ts @@ -76,6 +76,22 @@ if (fs.existsSync(networkFile)) { let program = new Command(); +function parseExtraArgs(variadicArgs?: string[]): { + extraArgs: string[]; + args: string[]; +} { + const rawArgs = process.argv.slice(2); + const dashDashIndex = rawArgs.indexOf("--"); + const extraArgs = + dashDashIndex !== -1 ? rawArgs.slice(dashDashIndex + 1) : []; + const args = variadicArgs + ? extraArgs.length > 0 + ? variadicArgs.slice(0, variadicArgs.length - extraArgs.length) + : variadicArgs + : []; + return { extraArgs, args }; +} + program.name("mops"); // --version @@ -275,18 +291,17 @@ program ), ) .allowUnknownOption(true) // TODO: restrict unknown before "--" - .action(async (canisters, options, command) => { + .action(async (canisters, options) => { checkConfigFile(true); - const extraArgsIndex = command.args.indexOf("--"); + const { extraArgs, args } = parseExtraArgs(canisters); await installAll({ silent: true, lock: "ignore", installFromLockFile: true, }); - await build(canisters.length ? canisters : undefined, { + await build(args.length ? args : undefined, { ...options, - extraArgs: - extraArgsIndex !== -1 ? command.args.slice(extraArgsIndex + 1) : [], + extraArgs, }); }); @@ -296,20 +311,18 @@ program .description("Check Motoko files for syntax errors and type issues") .option("--verbose", "Verbose console output") .addOption(new Option("--fix", "Apply autofixes")) - .addOption(new Option("--warnings", "Treat warnings as errors")) .allowUnknownOption(true) - .action(async (files, options, command) => { + .action(async (files, options) => { checkConfigFile(true); - const extraArgsIndex = command.args.indexOf("--"); + const { extraArgs, args: fileList } = parseExtraArgs(files); await installAll({ silent: true, lock: "ignore", installFromLockFile: true, }); - await check(files, { + await check(fileList, { ...options, - extraArgs: - extraArgsIndex !== -1 ? command.args.slice(extraArgsIndex + 1) : [], + extraArgs, }); }); @@ -710,13 +723,12 @@ program ), ) .allowUnknownOption(true) - .action(async (filter, options, command) => { + .action(async (filter, options) => { checkConfigFile(true); - const extraArgsIndex = command.args.indexOf("--"); + const { extraArgs } = parseExtraArgs(); await lint(filter, { ...options, - extraArgs: - extraArgsIndex !== -1 ? command.args.slice(extraArgsIndex + 1) : [], + extraArgs, }); }); diff --git a/cli/commands/check.ts b/cli/commands/check.ts index d1e8fdd4..004975cd 100644 --- a/cli/commands/check.ts +++ b/cli/commands/check.ts @@ -8,7 +8,6 @@ import { sourcesArgs } from "./sources.js"; export interface CheckOptions { verbose: boolean; fix: boolean; - warnings: boolean; extraArgs: string[]; } @@ -36,7 +35,6 @@ export async function check( "--check", file, ...sources.flat(), - ...(options.warnings ? ["-Werror"] : []), ...(options.extraArgs ?? []), ]; @@ -85,7 +83,6 @@ export async function check( "--check", file, ...sources.flat(), - ...(options.warnings ? ["-Werror"] : []), ...(options.extraArgs ?? []), ]; diff --git a/cli/tests/__snapshots__/build.test.ts.snap b/cli/tests/__snapshots__/build.test.ts.snap new file mode 100644 index 00000000..53040f8f --- /dev/null +++ b/cli/tests/__snapshots__/build.test.ts.snap @@ -0,0 +1,77 @@ +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing + +exports[`build error 1`] = ` +{ + "exitCode": 0, + "stderr": "", + "stdout": "build canister foo +moc-wrapper ["-c","--idl","-o",".mops/.build/foo.wasm","src/Foo.mo","--package","core",".mops/core@1.0.0/src","--release","--public-metadata","candid:service","--public-metadata","candid:args"] +Adding metadata to .mops/.build/foo.wasm + +✓ Built 1 canister successfully", +} +`; + +exports[`build error 2`] = ` +{ + "exitCode": 1, + "stderr": "Candid compatibility check failed for canister bar", + "stdout": "build canister bar", +} +`; + +exports[`build error 3`] = ` +{ + "exitCode": 1, + "stderr": "Candid compatibility check failed for canister bar", + "stdout": "build canister foo +build canister bar", +} +`; + +exports[`build ok 1`] = ` +{ + "exitCode": 0, + "stderr": "", + "stdout": "build canister foo +moc-wrapper ["-c","--idl","-o",".mops/.build/foo.wasm","src/Foo.mo","--package","core",".mops/core@1.0.0/src","--release","--public-metadata","candid:service","--public-metadata","candid:args"] +Adding metadata to .mops/.build/foo.wasm +build canister bar +moc-wrapper ["-c","--idl","-o",".mops/.build/bar.wasm","src/Bar.mo","--package","core",".mops/core@1.0.0/src","--release","--incremental-gc","--public-metadata","candid:service","--public-metadata","candid:args"] +Candid compatibility check passed for canister bar +Adding metadata to .mops/.build/bar.wasm + +✓ Built 2 canisters successfully", +} +`; + +exports[`build ok 2`] = ` +{ + "exitCode": 0, + "stderr": "", + "stdout": "build canister foo + +✓ Built 1 canister successfully", +} +`; + +exports[`build ok 3`] = ` +{ + "exitCode": 0, + "stderr": "", + "stdout": "build canister bar + +✓ Built 1 canister successfully", +} +`; + +exports[`build ok 4`] = ` +{ + "exitCode": 0, + "stderr": "", + "stdout": "build canister foo +build canister bar + +✓ Built 2 canisters successfully", +} +`; diff --git a/cli/tests/__snapshots__/check-candid.test.ts.snap b/cli/tests/__snapshots__/check-candid.test.ts.snap new file mode 100644 index 00000000..374ecfbc --- /dev/null +++ b/cli/tests/__snapshots__/check-candid.test.ts.snap @@ -0,0 +1,73 @@ +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing + +exports[`check-candid error 1`] = ` +{ + "exitCode": 1, + "stderr": "✖ Candid compatibility check failed", + "stdout": "", +} +`; + +exports[`check-candid error 2`] = ` +{ + "exitCode": 1, + "stderr": "✖ Candid compatibility check failed", + "stdout": "", +} +`; + +exports[`check-candid error 3`] = ` +{ + "exitCode": 1, + "stderr": "✖ Candid compatibility check failed", + "stdout": "", +} +`; + +exports[`check-candid error 4`] = ` +{ + "exitCode": 1, + "stderr": "✖ Candid compatibility check failed", + "stdout": "", +} +`; + +exports[`check-candid ok 1`] = ` +{ + "exitCode": 0, + "stderr": "", + "stdout": "✓ Candid compatibility check passed", +} +`; + +exports[`check-candid ok 2`] = ` +{ + "exitCode": 0, + "stderr": "", + "stdout": "✓ Candid compatibility check passed", +} +`; + +exports[`check-candid ok 3`] = ` +{ + "exitCode": 0, + "stderr": "", + "stdout": "✓ Candid compatibility check passed", +} +`; + +exports[`check-candid ok 4`] = ` +{ + "exitCode": 0, + "stderr": "", + "stdout": "✓ Candid compatibility check passed", +} +`; + +exports[`check-candid ok 5`] = ` +{ + "exitCode": 0, + "stderr": "", + "stdout": "✓ Candid compatibility check passed", +} +`; diff --git a/cli/tests/__snapshots__/check.test.ts.snap b/cli/tests/__snapshots__/check.test.ts.snap new file mode 100644 index 00000000..e1a365b5 --- /dev/null +++ b/cli/tests/__snapshots__/check.test.ts.snap @@ -0,0 +1,68 @@ +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing + +exports[`check error 1`] = ` +{ + "exitCode": 1, + "stderr": "Error.mo:1.1-5.2: type error [M0096], expression of type + actor {hello : shared () -> async Text} +cannot produce expected type + () +Error.mo:7.1-7.21: type error [M0057], unbound variable thisshouldnotcompile +✗ Check failed for file Error.mo (exit code: 1)", + "stdout": "", +} +`; + +exports[`check error 2`] = ` +{ + "exitCode": 1, + "stderr": "Ok.mo: No such file or directory +✗ Check failed for file Ok.mo (exit code: 1)", + "stdout": "", +} +`; + +exports[`check ok 1`] = ` +{ + "exitCode": 0, + "stderr": "", + "stdout": "✓ Ok.mo", +} +`; + +exports[`check ok 2`] = ` +{ + "exitCode": 0, + "stderr": "", + "stdout": "check Running moc: +moc-wrapper ["--check","Ok.mo","--package","core",".mops/core@2.0.0/src"] +✓ Ok.mo", +} +`; + +exports[`check warning 1`] = ` +{ + "exitCode": 0, + "stderr": "", + "stdout": "✓ M0223.mo", +} +`; + +exports[`check warning verbose 1`] = ` +{ + "exitCode": 0, + "stderr": "Warning.mo:3.9-3.15: warning [M0194], unused identifier unused (delete or rename to wildcard \`_\` or \`_unused\`)", + "stdout": "check Running moc: +moc-wrapper ["--check","Warning.mo","--package","core",".mops/core@2.0.0/src"] +✓ Warning.mo", +} +`; + +exports[`check warning with -Werror flag 1`] = ` +{ + "exitCode": 1, + "stderr": "Warning.mo:3.9-3.15: warning [M0194], unused identifier unused (delete or rename to wildcard \`_\` or \`_unused\`) +✗ Check failed for file Warning.mo (exit code: 1)", + "stdout": "", +} +`; diff --git a/cli/tests/__snapshots__/cli.test.ts.snap b/cli/tests/__snapshots__/cli.test.ts.snap deleted file mode 100644 index 2d68b232..00000000 --- a/cli/tests/__snapshots__/cli.test.ts.snap +++ /dev/null @@ -1,293 +0,0 @@ -// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing - -exports[`mops build error 1`] = ` -{ - "exitCode": 0, - "stderr": "", - "stdout": "build canister foo -moc-wrapper ["-c","--idl","-o",".mops/.build/foo.wasm","src/Foo.mo","--package","core",".mops/core@1.0.0/src","--release","--public-metadata","candid:service","--public-metadata","candid:args"] -Adding metadata to .mops/.build/foo.wasm - -✓ Built 1 canister successfully", -} -`; - -exports[`mops build error 2`] = ` -{ - "exitCode": 1, - "stderr": "Candid compatibility check failed for canister bar", - "stdout": "build canister bar", -} -`; - -exports[`mops build error 3`] = ` -{ - "exitCode": 1, - "stderr": "Candid compatibility check failed for canister bar", - "stdout": "build canister foo -build canister bar", -} -`; - -exports[`mops build success 1`] = ` -{ - "exitCode": 0, - "stderr": "", - "stdout": "build canister foo -moc-wrapper ["-c","--idl","-o",".mops/.build/foo.wasm","src/Foo.mo","--package","core",".mops/core@1.0.0/src","--release","--public-metadata","candid:service","--public-metadata","candid:args"] -Adding metadata to .mops/.build/foo.wasm -build canister bar -moc-wrapper ["-c","--idl","-o",".mops/.build/bar.wasm","src/Bar.mo","--package","core",".mops/core@1.0.0/src","--release","--incremental-gc","--public-metadata","candid:service","--public-metadata","candid:args"] -Candid compatibility check passed for canister bar -Adding metadata to .mops/.build/bar.wasm - -✓ Built 2 canisters successfully", -} -`; - -exports[`mops build success 2`] = ` -{ - "exitCode": 0, - "stderr": "", - "stdout": "build canister foo - -✓ Built 1 canister successfully", -} -`; - -exports[`mops build success 3`] = ` -{ - "exitCode": 0, - "stderr": "", - "stdout": "build canister bar - -✓ Built 1 canister successfully", -} -`; - -exports[`mops build success 4`] = ` -{ - "exitCode": 0, - "stderr": "", - "stdout": "build canister foo -build canister bar - -✓ Built 2 canisters successfully", -} -`; - -exports[`mops check error 1`] = ` -{ - "exitCode": 1, - "stderr": "Error.mo:1.1-5.2: type error [M0096], expression of type - actor {hello : shared () -> async Text} -cannot produce expected type - () -Error.mo:7.1-7.21: type error [M0057], unbound variable thisshouldnotcompile -✗ Check failed for file Error.mo (exit code: 1)", - "stdout": "", -} -`; - -exports[`mops check error 2`] = ` -{ - "exitCode": 1, - "stderr": "Ok.mo: No such file or directory -✗ Check failed for file Ok.mo (exit code: 1)", - "stdout": "", -} -`; - -exports[`mops check success 1`] = ` -{ - "exitCode": 0, - "stderr": "", - "stdout": "✓ Ok.mo", -} -`; - -exports[`mops check success 2`] = ` -{ - "exitCode": 0, - "stderr": "", - "stdout": "check Running moc: -moc-wrapper ["--check","Ok.mo","--package","core",".mops/core@2.0.0/src"] -✓ Ok.mo", -} -`; - -exports[`mops check warning 1`] = ` -{ - "exitCode": 0, - "stderr": "", - "stdout": "✓ M0223.mo", -} -`; - -exports[`mops check warning verbose 1`] = ` -{ - "exitCode": 0, - "stderr": "Warning.mo:3.9-3.15: warning [M0194], unused identifier unused (delete or rename to wildcard \`_\` or \`_unused\`)", - "stdout": "check Running moc: -moc-wrapper ["--check","Warning.mo","--package","core",".mops/core@2.0.0/src"] -✓ Warning.mo", -} -`; - -exports[`mops check warning with --warnings flag 1`] = ` -{ - "exitCode": 1, - "stderr": "Warning.mo:3.9-3.15: warning [M0194], unused identifier unused (delete or rename to wildcard \`_\` or \`_unused\`) -✗ Check failed for file Warning.mo (exit code: 1)", - "stdout": "", -} -`; - -exports[`mops check-candid 1`] = ` -{ - "exitCode": 0, - "stderr": "", - "stdout": "✓ Candid compatibility check passed", -} -`; - -exports[`mops check-candid 2`] = ` -{ - "exitCode": 0, - "stderr": "", - "stdout": "✓ Candid compatibility check passed", -} -`; - -exports[`mops check-candid 3`] = ` -{ - "exitCode": 0, - "stderr": "", - "stdout": "✓ Candid compatibility check passed", -} -`; - -exports[`mops check-candid 4`] = ` -{ - "exitCode": 0, - "stderr": "", - "stdout": "✓ Candid compatibility check passed", -} -`; - -exports[`mops check-candid 5`] = ` -{ - "exitCode": 0, - "stderr": "", - "stdout": "✓ Candid compatibility check passed", -} -`; - -exports[`mops check-candid 6`] = ` -{ - "exitCode": 1, - "stderr": "✖ Candid compatibility check failed", - "stdout": "", -} -`; - -exports[`mops check-candid 7`] = ` -{ - "exitCode": 1, - "stderr": "✖ Candid compatibility check failed", - "stdout": "", -} -`; - -exports[`mops check-candid 8`] = ` -{ - "exitCode": 1, - "stderr": "✖ Candid compatibility check failed", - "stdout": "", -} -`; - -exports[`mops check-candid 9`] = ` -{ - "exitCode": 1, - "stderr": "✖ Candid compatibility check failed", - "stdout": "", -} -`; - -exports[`mops lint 1`] = ` -{ - "exitCode": 1, - "stderr": " × [ERROR]: no-bool-switch - ╭─[/lint/src/NoBoolSwitch.mo:3:5] - 2 │ public func boolSwitch(b : Bool) : Bool { - 3 │ ╭─▶ switch (b) { - 4 │ │ case false { false }; - 5 │ │ case true { true }; - 6 │ ├─▶ }; - · ╰──── Don't switch on boolean values, use if instead - 7 │ }; - ╰──── - -Error: Found 1 errors -Lint failed with exit code 1", - "stdout": "lint Running lintoko: -/lintoko/0.7.0/lintoko -["--verbose","--rules","lints","/lint/src/Ok.mo","/lint/src/NoBoolSwitch.mo"] -DEBUG file input: /lint/src/Ok.mo -DEBUG file input: /lint/src/NoBoolSwitch.mo -DEBUG Loading rules from: lints -DEBUG Parsing extra rule at: lints/no-bool-switch.toml -DEBUG Linting file: /lint/src/NoBoolSwitch.mo -DEBUG Linting file: /lint/src/Ok.mo", -} -`; - -exports[`mops lint 2`] = ` -{ - "exitCode": 0, - "stderr": "", - "stdout": "lint Running lintoko: -/lintoko/0.7.0/lintoko -["--verbose","--rules","lints","/lint/src/Ok.mo"] -DEBUG file input: /lint/src/Ok.mo -DEBUG Loading rules from: lints -DEBUG Parsing extra rule at: lints/no-bool-switch.toml -DEBUG Linting file: /lint/src/Ok.mo -✓ Lint succeeded", -} -`; - -exports[`mops lint 3`] = ` -{ - "exitCode": 1, - "stderr": " × [ERROR]: no-bool-switch - ╭─[/lint/src/NoBoolSwitch.mo:3:5] - 2 │ public func boolSwitch(b : Bool) : Bool { - 3 │ ╭─▶ switch (b) { - 4 │ │ case false { false }; - 5 │ │ case true { true }; - 6 │ ├─▶ }; - · ╰──── Don't switch on boolean values, use if instead - 7 │ }; - ╰──── - -Error: Found 1 errors -Lint failed with exit code 1", - "stdout": "lint Running lintoko: -/lintoko/0.7.0/lintoko -["--verbose","--rules","lints","/lint/src/NoBoolSwitch.mo"] -DEBUG file input: /lint/src/NoBoolSwitch.mo -DEBUG Loading rules from: lints -DEBUG Parsing extra rule at: lints/no-bool-switch.toml -DEBUG Linting file: /lint/src/NoBoolSwitch.mo", -} -`; - -exports[`mops lint 4`] = ` -{ - "exitCode": 1, - "stderr": "No files found for filter 'DoesNotExist'", - "stdout": "", -} -`; diff --git a/cli/tests/__snapshots__/lint.test.ts.snap b/cli/tests/__snapshots__/lint.test.ts.snap new file mode 100644 index 00000000..79adc59f --- /dev/null +++ b/cli/tests/__snapshots__/lint.test.ts.snap @@ -0,0 +1,78 @@ +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing + +exports[`lint error 1`] = ` +{ + "exitCode": 1, + "stderr": " × [ERROR]: no-bool-switch + ╭─[/lint/src/NoBoolSwitch.mo:3:5] + 2 │ public func boolSwitch(b : Bool) : Bool { + 3 │ ╭─▶ switch (b) { + 4 │ │ case false { false }; + 5 │ │ case true { true }; + 6 │ ├─▶ }; + · ╰──── Don't switch on boolean values, use if instead + 7 │ }; + ╰──── + +Error: Found 1 errors +Lint failed with exit code 1", + "stdout": "lint Running lintoko: +/lintoko/0.7.0/lintoko +["--verbose","--rules","lints","/lint/src/Ok.mo","/lint/src/NoBoolSwitch.mo"] +DEBUG file input: /lint/src/Ok.mo +DEBUG file input: /lint/src/NoBoolSwitch.mo +DEBUG Loading rules from: lints +DEBUG Parsing extra rule at: lints/no-bool-switch.toml +DEBUG Linting file: /lint/src/NoBoolSwitch.mo +DEBUG Linting file: /lint/src/Ok.mo", +} +`; + +exports[`lint error 2`] = ` +{ + "exitCode": 1, + "stderr": " × [ERROR]: no-bool-switch + ╭─[/lint/src/NoBoolSwitch.mo:3:5] + 2 │ public func boolSwitch(b : Bool) : Bool { + 3 │ ╭─▶ switch (b) { + 4 │ │ case false { false }; + 5 │ │ case true { true }; + 6 │ ├─▶ }; + · ╰──── Don't switch on boolean values, use if instead + 7 │ }; + ╰──── + +Error: Found 1 errors +Lint failed with exit code 1", + "stdout": "lint Running lintoko: +/lintoko/0.7.0/lintoko +["--verbose","--rules","lints","/lint/src/NoBoolSwitch.mo"] +DEBUG file input: /lint/src/NoBoolSwitch.mo +DEBUG Loading rules from: lints +DEBUG Parsing extra rule at: lints/no-bool-switch.toml +DEBUG Linting file: /lint/src/NoBoolSwitch.mo", +} +`; + +exports[`lint error 3`] = ` +{ + "exitCode": 1, + "stderr": "No files found for filter 'DoesNotExist'", + "stdout": "", +} +`; + +exports[`lint ok 1`] = ` +{ + "exitCode": 0, + "stderr": "", + "stdout": "lint Running lintoko: +/lintoko/0.7.0/lintoko +["--verbose","--rules","lints","/lint/src/Ok.mo"] +DEBUG file input: /lint/src/Ok.mo +DEBUG Loading rules from: lints +DEBUG Parsing extra rule at: lints/no-bool-switch.toml +DEBUG Linting file: /lint/src/Ok.mo +✓ Lint succeeded", +} +`; diff --git a/cli/tests/build.test.ts b/cli/tests/build.test.ts new file mode 100644 index 00000000..fcdc637a --- /dev/null +++ b/cli/tests/build.test.ts @@ -0,0 +1,24 @@ +import { describe, expect, test } from "@jest/globals"; +import path from "path"; +import { cliSnapshot } from "./helpers"; + +describe("build", () => { + test("ok", async () => { + const cwd = path.join(import.meta.dirname, "build/success"); + await cliSnapshot(["build", "--verbose"], { cwd }, 0); + await cliSnapshot(["build", "foo"], { cwd }, 0); + await cliSnapshot(["build", "bar"], { cwd }, 0); + await cliSnapshot(["build", "foo", "bar"], { cwd }, 0); + }); + + test("error", async () => { + const cwd = path.join(import.meta.dirname, "build/error"); + await cliSnapshot(["build", "foo", "--verbose"], { cwd }, 0); + expect((await cliSnapshot(["build", "bar"], { cwd }, 1)).stderr).toMatch( + "Candid compatibility check failed for canister bar", + ); + expect( + (await cliSnapshot(["build", "foo", "bar"], { cwd }, 1)).stderr, + ).toMatch("Candid compatibility check failed for canister bar"); + }); +}); diff --git a/cli/tests/check-candid.test.ts b/cli/tests/check-candid.test.ts new file mode 100644 index 00000000..dfffcd2e --- /dev/null +++ b/cli/tests/check-candid.test.ts @@ -0,0 +1,22 @@ +import { describe, test } from "@jest/globals"; +import path from "path"; +import { cliSnapshot } from "./helpers"; + +describe("check-candid", () => { + test("ok", async () => { + const cwd = path.join(import.meta.dirname, "check-candid"); + await cliSnapshot(["check-candid", "a.did", "a.did"], { cwd }, 0); + await cliSnapshot(["check-candid", "b.did", "b.did"], { cwd }, 0); + await cliSnapshot(["check-candid", "c.did", "c.did"], { cwd }, 0); + await cliSnapshot(["check-candid", "a.did", "b.did"], { cwd }, 0); + await cliSnapshot(["check-candid", "b.did", "a.did"], { cwd }, 0); + }); + + test("error", async () => { + const cwd = path.join(import.meta.dirname, "check-candid"); + await cliSnapshot(["check-candid", "a.did", "c.did"], { cwd }, 1); + await cliSnapshot(["check-candid", "c.did", "a.did"], { cwd }, 1); + await cliSnapshot(["check-candid", "b.did", "c.did"], { cwd }, 1); + await cliSnapshot(["check-candid", "c.did", "b.did"], { cwd }, 1); + }); +}); diff --git a/cli/tests/check.test.ts b/cli/tests/check.test.ts new file mode 100644 index 00000000..2d99fe7b --- /dev/null +++ b/cli/tests/check.test.ts @@ -0,0 +1,92 @@ +import { beforeEach, describe, expect, test } from "@jest/globals"; +import { cpSync, readdirSync, readFileSync, unlinkSync } from "node:fs"; +import path from "path"; +import { cli, cliSnapshot } from "./helpers"; + +describe("check", () => { + test("ok", async () => { + const cwd = path.join(import.meta.dirname, "check/success"); + await cliSnapshot(["check", "Ok.mo"], { cwd }, 0); + await cliSnapshot(["check", "Ok.mo", "--verbose"], { cwd }, 0); + }); + + test("error", async () => { + const cwd = path.join(import.meta.dirname, "check/error"); + await cliSnapshot(["check", "Error.mo"], { cwd }, 1); + await cliSnapshot(["check", "Ok.mo", "Error.mo"], { cwd }, 1); + }); + + test("warning", async () => { + const cwd = path.join(import.meta.dirname, "check/fix"); + await cliSnapshot(["check", "M0223.mo"], { cwd }, 0); + }); + + test("warning verbose", async () => { + const cwd = path.join(import.meta.dirname, "check/success"); + const result = await cliSnapshot( + ["check", "Warning.mo", "--verbose"], + { cwd }, + 0, + ); + expect(result.stderr).toMatch(/warning \[M0194\]/); + expect(result.stderr).toMatch(/unused identifier/); + }); + + test("warning with -Werror flag", async () => { + const cwd = path.join(import.meta.dirname, "check/success"); + await cliSnapshot(["check", "Warning.mo", "--", "-Werror"], { cwd }, 1); + }); + + describe("--fix", () => { + const fixDir = path.join(import.meta.dirname, "check/fix"); + const runDir = path.join(fixDir, "run"); + + beforeEach(() => { + for (const file of readdirSync(runDir).filter((f) => f.endsWith(".mo"))) { + unlinkSync(path.join(runDir, file)); + } + }); + + async function checkFix( + errorCode: string, + original: string, + expected: string, + ) { + const file = `${errorCode}.mo`; + cpSync(path.join(fixDir, file), path.join(runDir, file)); + const before = readFileSync(path.join(runDir, file), "utf-8"); + expect(before).toContain(original); + const result = await cli(["check", `run/${file}`, "--fix"], { + cwd: fixDir, + }); + expect(result.exitCode).toBe(0); + const after = readFileSync(path.join(runDir, file), "utf-8"); + expect(after).toContain(expected); + expect(after).not.toContain(original); + } + + test("M0223", async () => { + await checkFix("M0223", "List.empty()", "List.empty()"); + }); + + test("M0236", async () => { + await checkFix("M0236", "List.sortInPlace(list)", "list.sortInPlace()"); + }); + + test("M0237", async () => { + await checkFix( + "M0237", + "list.sortInPlace(Nat.compare)", + "list.sortInPlace()", + ); + }); + + test("verbose", async () => { + const result = await cli(["check", "Ok.mo", "--fix", "--verbose"], { + cwd: fixDir, + }); + expect(result.exitCode).toBe(0); + expect(result.stdout).toBeTruthy(); + }); + }); +}); diff --git a/cli/tests/check/fix/M0223.mo b/cli/tests/check/fix/M0223.mo index d3f52c61..624fc1d0 100644 --- a/cli/tests/check/fix/M0223.mo +++ b/cli/tests/check/fix/M0223.mo @@ -1,7 +1,10 @@ // M0223: Redundant type instantiation // The type annotation is not needed when it can be inferred +import List "mo:core/List"; + persistent actor { - public func testM0223() : async Nat { - let _x : Nat = 42; + public func testM0223() : async Bool { + let list : List.List = List.empty(); + list.isEmpty(); }; }; diff --git a/cli/tests/check/fix/M0236.mo b/cli/tests/check/fix/M0236.mo index 671d3227..fc1873ef 100644 --- a/cli/tests/check/fix/M0236.mo +++ b/cli/tests/check/fix/M0236.mo @@ -1,11 +1,11 @@ // M0236: Suggested to use dot notation // Function calls can be rewritten using dot notation -import Array "mo:core/Array"; +import List "mo:core/List"; +import Nat "mo:core/Nat"; persistent actor { - public func testM0236() : async Nat { - let arr = [1, 2, 3]; - let len = Array.size(arr); - len; + public func testM0236() : async () { + let list = List.fromArray([1, 2, 3]); + List.sortInPlace(list); }; }; diff --git a/cli/tests/check/fix/M0237.mo b/cli/tests/check/fix/M0237.mo index b04bc62a..c5d192ef 100644 --- a/cli/tests/check/fix/M0237.mo +++ b/cli/tests/check/fix/M0237.mo @@ -1,7 +1,11 @@ // M0237: Redundant explicit implicit argument // Some arguments can be inferred and don't need to be specified +import List "mo:core/List"; +import Nat "mo:core/Nat"; + persistent actor { public func testM0237() : async () { - let _x : ?Text = null; + let list = List.fromArray([3, 2, 1]); + list.sortInPlace(Nat.compare); }; }; diff --git a/cli/tests/cli.test.ts b/cli/tests/cli.test.ts index 33afce31..dd0bf68f 100644 --- a/cli/tests/cli.test.ts +++ b/cli/tests/cli.test.ts @@ -1,189 +1,12 @@ import { describe, expect, test } from "@jest/globals"; -import { cpSync, mkdirSync, readFileSync } from "node:fs"; -import { execa } from "execa"; -import path, { dirname } from "path"; -import { fileURLToPath } from "url"; +import { cli } from "./helpers"; -interface CliOptions { - cwd?: string; -} - -const cli = async (args: string[], { cwd }: CliOptions = {}) => { - return await execa("npm", ["run", "--silent", "mops", "--", ...args], { - env: { ...process.env, ...(cwd != null && { MOPS_CWD: cwd }) }, - ...(cwd != null && { cwd }), - stdio: "pipe", - reject: false, - }); -}; - -// Strip ANSI escape codes for portable snapshots (avoid control char in regex literal) -const stripAnsi = (s: string) => - s.replace(new RegExp(`\u001b\\[[0-9;]*m`, "g"), ""); - -const normalizePaths = (text: string): string => { - // Replace absolute paths with placeholders for CI - return stripAnsi( - text - .replaceAll(dirname(fileURLToPath(import.meta.url)), "") - .replace(/\/[^\s"]+\/\.cache\/mops/g, "") - .replace(/\/[^\s"]+\/Library\/Caches\/mops/g, "") - .replace(/\/[^\s"[\]]+\/moc(?:-wrapper)?(?=\s|$)/g, "moc-wrapper") - .replace(/\/[^\s"[\]]+\.motoko\/bin\/moc/g, "moc-wrapper"), - ); -}; - -const cliSnapshot = async ( - args: string[], - options: CliOptions, - exitCode: number, -) => { - const result = await cli(args, options); - expect({ - command: result.command, - exitCode: result.exitCode, - timedOut: result.timedOut, - stdio: Boolean(result.stdout || result.stderr), - }).toEqual({ - command: result.command, - exitCode, - timedOut: false, - stdio: true, - }); - expect({ - exitCode: result.exitCode, - stdout: normalizePaths(result.stdout), - stderr: normalizePaths(result.stderr), - }).toMatchSnapshot(); - return result; -}; - -describe("mops", () => { - test("version", async () => { +describe("cli", () => { + test("--version", async () => { expect((await cli(["--version"])).stdout).toMatch(/CLI \d+\.\d+\.\d+/); }); - test("help", async () => { + test("--help", async () => { expect((await cli(["--help"])).stdout).toMatch(/^Usage: mops/m); }); - - test("build success", async () => { - const cwd = path.join(import.meta.dirname, "build/success"); - await cliSnapshot(["build", "--verbose"], { cwd }, 0); - await cliSnapshot(["build", "foo"], { cwd }, 0); - await cliSnapshot(["build", "bar"], { cwd }, 0); - await cliSnapshot(["build", "foo", "bar"], { cwd }, 0); - }); - - test("build error", async () => { - const cwd = path.join(import.meta.dirname, "build/error"); - await cliSnapshot(["build", "foo", "--verbose"], { cwd }, 0); - expect((await cliSnapshot(["build", "bar"], { cwd }, 1)).stderr).toMatch( - "Candid compatibility check failed for canister bar", - ); - expect( - (await cliSnapshot(["build", "foo", "bar"], { cwd }, 1)).stderr, - ).toMatch("Candid compatibility check failed for canister bar"); - }); - - test("check success", async () => { - const cwd = path.join(import.meta.dirname, "check/success"); - await cliSnapshot(["check", "Ok.mo"], { cwd }, 0); - await cliSnapshot(["check", "Ok.mo", "--verbose"], { cwd }, 0); - }); - - test("check error", async () => { - const cwd = path.join(import.meta.dirname, "check/error"); - await cliSnapshot(["check", "Error.mo"], { cwd }, 1); - await cliSnapshot(["check", "Ok.mo", "Error.mo"], { cwd }, 1); - }); - - test("check warning", async () => { - const cwd = path.join(import.meta.dirname, "check/fix"); - await cliSnapshot(["check", "M0223.mo"], { cwd }, 0); - }); - - test("check warning verbose", async () => { - const cwd = path.join(import.meta.dirname, "check/success"); - const result = await cliSnapshot( - ["check", "Warning.mo", "--verbose"], - { cwd }, - 0, - ); - // Verify the warning is shown in stderr without --warnings flag - expect(result.stderr).toMatch(/warning \[M0194\]/); - expect(result.stderr).toMatch(/unused identifier/); - }); - - test("check warning with --warnings flag", async () => { - const cwd = path.join(import.meta.dirname, "check/success"); - // With --warnings flag, warnings should cause check to fail - await cliSnapshot(["check", "Warning.mo", "--warnings"], { cwd }, 1); - }); - - const fixDir = path.join(import.meta.dirname, "check/fix"); - const runDir = path.join(fixDir, "run"); - - async function checkFix(file: string, original: string, expected: string) { - mkdirSync(runDir, { recursive: true }); - cpSync(path.join(fixDir, file), path.join(runDir, file)); - const before = readFileSync(path.join(runDir, file), "utf-8"); - expect(before).toContain(original); - const result = await cli(["check", `run/${file}`, "--fix"], { - cwd: fixDir, - }); - expect(result.exitCode).toBe(0); - const after = readFileSync(path.join(runDir, file), "utf-8"); - expect(after).toContain(expected); - expect(after).not.toContain(original); - } - - test("check --fix M0223", async () => { - await checkFix("M0223.mo", "let _x : Nat = 42", "let _x = 42"); - }); - - test("check --fix M0236", async () => { - await checkFix("M0236.mo", "Array.size(arr)", "arr.size()"); - }); - - test("check --fix M0237", async () => { - await checkFix("M0237.mo", "let _x : ?Text = null", "let _x = null"); - }); - - test("check --fix verbose", async () => { - const cwd = path.join(import.meta.dirname, "check/fix"); - const result = await cli(["check", "Ok.mo", "--fix", "--verbose"], { - cwd, - }); - expect(result.exitCode).toBe(0); - expect(result.stdout).toBeTruthy(); - }); - - test("check-candid", async () => { - const cwd = path.join(import.meta.dirname, "check-candid"); - await cliSnapshot(["check-candid", "a.did", "a.did"], { cwd }, 0); - await cliSnapshot(["check-candid", "b.did", "b.did"], { cwd }, 0); - await cliSnapshot(["check-candid", "c.did", "c.did"], { cwd }, 0); - await cliSnapshot(["check-candid", "a.did", "b.did"], { cwd }, 0); - await cliSnapshot(["check-candid", "b.did", "a.did"], { cwd }, 0); - await cliSnapshot(["check-candid", "a.did", "c.did"], { cwd }, 1); - await cliSnapshot(["check-candid", "c.did", "a.did"], { cwd }, 1); - await cliSnapshot(["check-candid", "b.did", "c.did"], { cwd }, 1); - await cliSnapshot(["check-candid", "c.did", "b.did"], { cwd }, 1); - }); - - test("lint", async () => { - const cwd = path.join(import.meta.dirname, "lint"); - await cliSnapshot(["lint", "--verbose"], { cwd }, 1); - await cliSnapshot(["lint", "Ok", "--verbose"], { cwd }, 0); - await cliSnapshot(["lint", "NoBoolSwitch", "--verbose"], { cwd }, 1); - await cliSnapshot(["lint", "DoesNotExist"], { cwd }, 1); - }); - - test("toolchain file URI", async () => { - const cwd = path.join(import.meta.dirname, "toolchain"); - const result = await cli(["toolchain", "bin", "moc"], { cwd }); - expect(result.exitCode).toBe(0); - expect(result.stdout.trim()).toBe("./mock"); - }); }); diff --git a/cli/tests/helpers.ts b/cli/tests/helpers.ts new file mode 100644 index 00000000..1eb312aa --- /dev/null +++ b/cli/tests/helpers.ts @@ -0,0 +1,58 @@ +import { expect } from "@jest/globals"; +import { execa } from "execa"; +import { dirname } from "path"; +import { fileURLToPath } from "url"; + +export interface CliOptions { + cwd?: string; +} + +export const cli = async (args: string[], { cwd }: CliOptions = {}) => { + return await execa("npm", ["run", "--silent", "mops", "--", ...args], { + env: { ...process.env, ...(cwd != null && { MOPS_CWD: cwd }) }, + ...(cwd != null && { cwd }), + stdio: "pipe", + reject: false, + }); +}; + +// Strip ANSI escape codes for portable snapshots (avoid control char in regex literal) +const stripAnsi = (s: string) => + s.replace(new RegExp(`\u001b\\[[0-9;]*m`, "g"), ""); + +const normalizePaths = (text: string): string => { + // Replace absolute paths with placeholders for CI + return stripAnsi( + text + .replaceAll(dirname(fileURLToPath(import.meta.url)), "") + .replace(/\/[^\s"]+\/\.cache\/mops/g, "") + .replace(/\/[^\s"]+\/Library\/Caches\/mops/g, "") + .replace(/\/[^\s"[\]]+\/moc(?:-wrapper)?(?=\s|$)/g, "moc-wrapper") + .replace(/\/[^\s"[\]]+\.motoko\/bin\/moc/g, "moc-wrapper"), + ); +}; + +export const cliSnapshot = async ( + args: string[], + options: CliOptions, + exitCode: number, +) => { + const result = await cli(args, options); + expect({ + command: result.command, + exitCode: result.exitCode, + timedOut: result.timedOut, + stdio: Boolean(result.stdout || result.stderr), + }).toEqual({ + command: result.command, + exitCode, + timedOut: false, + stdio: true, + }); + expect({ + exitCode: result.exitCode, + stdout: normalizePaths(result.stdout), + stderr: normalizePaths(result.stderr), + }).toMatchSnapshot(); + return result; +}; diff --git a/cli/tests/lint.test.ts b/cli/tests/lint.test.ts new file mode 100644 index 00000000..2062fe7d --- /dev/null +++ b/cli/tests/lint.test.ts @@ -0,0 +1,17 @@ +import { describe, test } from "@jest/globals"; +import path from "path"; +import { cliSnapshot } from "./helpers"; + +describe("lint", () => { + test("ok", async () => { + const cwd = path.join(import.meta.dirname, "lint"); + await cliSnapshot(["lint", "Ok", "--verbose"], { cwd }, 0); + }); + + test("error", async () => { + const cwd = path.join(import.meta.dirname, "lint"); + await cliSnapshot(["lint", "--verbose"], { cwd }, 1); + await cliSnapshot(["lint", "NoBoolSwitch", "--verbose"], { cwd }, 1); + await cliSnapshot(["lint", "DoesNotExist"], { cwd }, 1); + }); +}); diff --git a/cli/tests/toolchain.test.ts b/cli/tests/toolchain.test.ts new file mode 100644 index 00000000..1f07fd07 --- /dev/null +++ b/cli/tests/toolchain.test.ts @@ -0,0 +1,12 @@ +import { describe, expect, test } from "@jest/globals"; +import path from "path"; +import { cli } from "./helpers"; + +describe("toolchain", () => { + test("file URI", async () => { + const cwd = path.join(import.meta.dirname, "toolchain"); + const result = await cli(["toolchain", "bin", "moc"], { cwd }); + expect(result.exitCode).toBe(0); + expect(result.stdout.trim()).toBe("./mock"); + }); +}); From c19f4d3a8160ece6dfaadc3f8dbda4eba5b2f3c1 Mon Sep 17 00:00:00 2001 From: rvanasa Date: Wed, 18 Feb 2026 13:27:41 -0500 Subject: [PATCH 6/6] Update M0223 test case --- cli/commands/check.ts | 35 +------ cli/helpers/autofix-motoko.ts | 113 +++++++++++++-------- cli/tests/__snapshots__/check.test.ts.snap | 4 +- cli/tests/check.test.ts | 29 ++++-- cli/tests/check/fix/M0223.mo | 9 +- 5 files changed, 104 insertions(+), 86 deletions(-) diff --git a/cli/commands/check.ts b/cli/commands/check.ts index 004975cd..7d743854 100644 --- a/cli/commands/check.ts +++ b/cli/commands/check.ts @@ -23,22 +23,15 @@ export async function check( let mocPath = getMocPath(); let sources = await sourcesArgs(); + const mocArgs = ["--check", ...sources.flat(), ...(options.extraArgs ?? [])]; - // Helper function to compile and get errors const compileErrors = async ( filesToCheck: string[], ): Promise => { let allErrors = ""; for (const file of filesToCheck) { - let args = [ - "--check", - file, - ...sources.flat(), - ...(options.extraArgs ?? []), - ]; - - const result = await execa(mocPath, args, { + const result = await execa(mocPath, [file, ...mocArgs], { stdio: "pipe", reject: false, }); @@ -54,7 +47,6 @@ export async function check( return allErrors.trim() ? allErrors.trim() : null; }; - // If fix flag is enabled, attempt to fix errors if (options.fix) { if (options.verbose) { console.log(chalk.blue("check"), chalk.gray("Attempting to fix files")); @@ -77,44 +69,25 @@ export async function check( } } - // Final check to verify all files pass for (const file of fileList) { - let args = [ - "--check", - file, - ...sources.flat(), - ...(options.extraArgs ?? []), - ]; - try { + const args = [file, ...mocArgs]; if (options.verbose) { console.log(chalk.blue("check"), chalk.gray("Running moc:")); console.log(chalk.gray(mocPath, JSON.stringify(args))); } const result = await execa(mocPath, args, { - stdio: options.verbose ? "inherit" : "pipe", + stdio: "inherit", reject: false, }); if (result.exitCode !== 0) { - if (!options.verbose) { - if (result.stderr) { - console.error(chalk.red(result.stderr)); - } - if (result.stdout?.trim()) { - console.error(result.stdout); - } - } cliError( `✗ Check failed for file ${file} (exit code: ${result.exitCode})`, ); } - if (options.verbose && result.stdout && result.stdout.trim()) { - console.log(result.stdout); - } - console.log(chalk.green(`✓ ${file}`)); } catch (err: any) { cliError( diff --git a/cli/helpers/autofix-motoko.ts b/cli/helpers/autofix-motoko.ts index b1096b5f..bdc76fdf 100644 --- a/cli/helpers/autofix-motoko.ts +++ b/cli/helpers/autofix-motoko.ts @@ -229,19 +229,17 @@ export class MotokoFixer { const range = rangeFromDiagnostic(diag); const originalText = fileContent.textAt(range); - // Extract function name and first argument from the error message - // The message format is typically: "suggestion: use dot notation: obj.method(...)" - const match = originalText.match(/(\w+)\s*\(\s*(\w+)/); + // Match optional module prefix, method name, and first argument + // e.g. "List.sortInPlace(list, ...)" -> "list.sortInPlace(...)" + const match = originalText.match(/(?:\w+\.)*(\w+)\s*\(\s*(\w+)/); if (!match) { return null; } - const funcName = match[1]; - - // Simple replacement: convert func(obj, ...) to obj.func(...) + // Replace: [Module.]funcName(obj, ...) -> obj.funcName(...) const newText = originalText.replace( - /(\w+)\s*\(\s*(\w+)/, - `$2.${funcName}(`, + /(?:\w+\.)*(\w+)\s*\(\s*(\w+)/, + `$2.$1(`, ); return { range, newText }; @@ -419,29 +417,21 @@ export class MotokoFixer { } } -export async function autofixMotoko( - files: string[], - compileErrors: (filePaths: string[]) => Promise, - baseDir: string = process.cwd(), +export type FileSet = Record; + +export async function motokoFix( + files: FileSet, + errors: string, + compileErrors: (fileSet: FileSet) => Promise, ): Promise<{ - fixedCount: number; + fixedFiles: FileSet; fixedErrorCounts: Record; + result: { errors: string | null; files: FileSet }; } | null> { - // Load file contents - const fileContents = new Map(); - const filePaths = new Map(); // maps relative path to absolute path - - for (const file of files) { - const absolutePath = file.startsWith("/") ? file : join(baseDir, file); - const content = readFileSync(absolutePath, "utf-8"); - fileContents.set(file, content); - filePaths.set(file, absolutePath); - } - - let currentFiles = fileContents; - let currentErrors = await compileErrors(files); - let totalFixedCount = 0; + const currentFiles = new Map(Object.entries(files)); + let currentErrors: string | null = errors; const allFixedErrorCounts: Record = {}; + const changedFiles = new Set(); const fixer = new MotokoFixer(); const maxIterations = 10; @@ -452,41 +442,82 @@ export async function autofixMotoko( break; } - // Merge fixed files for (const [file, content] of fixResult.fixedFiles) { currentFiles.set(file, content); + changedFiles.add(file); } - totalFixedCount += fixResult.fixedFiles.size; - - // Accumulate fixed error counts for (const [code, count] of Object.entries(fixResult.fixedErrorCounts)) { allFixedErrorCounts[code] = (allFixedErrorCounts[code] ?? 0) + count; } - currentErrors = await compileErrors(files); + currentErrors = await compileErrors(Object.fromEntries(currentFiles)); if (currentErrors !== null) { fixer.verifyFixes(currentErrors); } } - if (totalFixedCount === 0) { + if (changedFiles.size === 0) { return null; } - // Write fixed files back to disk + const fixedFiles: FileSet = {}; + for (const file of changedFiles) { + const content = currentFiles.get(file); + if (content !== undefined) { + fixedFiles[file] = content; + } + } + + return { + fixedFiles, + fixedErrorCounts: allFixedErrorCounts, + result: { + errors: currentErrors, + files: Object.fromEntries(currentFiles), + }, + }; +} + +export async function autofixMotoko( + files: string[], + compileErrors: (filePaths: string[]) => Promise, + baseDir: string = process.cwd(), +): Promise<{ + fixedCount: number; + fixedErrorCounts: Record; +} | null> { + const fileContents: FileSet = {}; + const filePaths = new Map(); + for (const file of files) { - const absolutePath = filePaths.get(file); - if (absolutePath && currentFiles.has(file)) { - const fixedContent = currentFiles.get(file); - if (fixedContent) { - writeFileSync(absolutePath, fixedContent, "utf-8"); + const absolutePath = file.startsWith("/") ? file : join(baseDir, file); + fileContents[file] = readFileSync(absolutePath, "utf-8"); + filePaths.set(file, absolutePath); + } + + const initialErrors = await compileErrors(files); + if (initialErrors === null) { + return null; + } + + const fix = await motokoFix(fileContents, initialErrors, async (fileSet) => { + // Write updated in-memory files to disk before compiling + for (const [file, content] of Object.entries(fileSet)) { + const absolutePath = filePaths.get(file); + if (absolutePath) { + writeFileSync(absolutePath, content, "utf-8"); } } + return compileErrors(files); + }); + + if (!fix) { + return null; } return { - fixedCount: totalFixedCount, - fixedErrorCounts: allFixedErrorCounts, + fixedCount: Object.keys(fix.fixedFiles).length, + fixedErrorCounts: fix.fixedErrorCounts, }; } diff --git a/cli/tests/__snapshots__/check.test.ts.snap b/cli/tests/__snapshots__/check.test.ts.snap index e1a365b5..1320ebbb 100644 --- a/cli/tests/__snapshots__/check.test.ts.snap +++ b/cli/tests/__snapshots__/check.test.ts.snap @@ -35,7 +35,7 @@ exports[`check ok 2`] = ` "exitCode": 0, "stderr": "", "stdout": "check Running moc: -moc-wrapper ["--check","Ok.mo","--package","core",".mops/core@2.0.0/src"] +moc-wrapper ["Ok.mo","--check","--package","core",".mops/core@2.0.0/src"] ✓ Ok.mo", } `; @@ -53,7 +53,7 @@ exports[`check warning verbose 1`] = ` "exitCode": 0, "stderr": "Warning.mo:3.9-3.15: warning [M0194], unused identifier unused (delete or rename to wildcard \`_\` or \`_unused\`)", "stdout": "check Running moc: -moc-wrapper ["--check","Warning.mo","--package","core",".mops/core@2.0.0/src"] +moc-wrapper ["Warning.mo","--check","--package","core",".mops/core@2.0.0/src"] ✓ Warning.mo", } `; diff --git a/cli/tests/check.test.ts b/cli/tests/check.test.ts index 2d99fe7b..b8de8c05 100644 --- a/cli/tests/check.test.ts +++ b/cli/tests/check.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, test } from "@jest/globals"; +import { beforeAll, describe, expect, test } from "@jest/globals"; import { cpSync, readdirSync, readFileSync, unlinkSync } from "node:fs"; import path from "path"; import { cli, cliSnapshot } from "./helpers"; @@ -41,7 +41,7 @@ describe("check", () => { const fixDir = path.join(import.meta.dirname, "check/fix"); const runDir = path.join(fixDir, "run"); - beforeEach(() => { + beforeAll(() => { for (const file of readdirSync(runDir).filter((f) => f.endsWith(".mo"))) { unlinkSync(path.join(runDir, file)); } @@ -53,12 +53,25 @@ describe("check", () => { expected: string, ) { const file = `${errorCode}.mo`; - cpSync(path.join(fixDir, file), path.join(runDir, file)); - const before = readFileSync(path.join(runDir, file), "utf-8"); + const runFilePath = path.join(runDir, file); + cpSync(path.join(fixDir, file), runFilePath); + const before = readFileSync(runFilePath, "utf-8"); expect(before).toContain(original); - const result = await cli(["check", `run/${file}`, "--fix"], { - cwd: fixDir, - }); + const result = await cli( + [ + "check", + runFilePath, + "--fix", + "--", + "--ai-errors", + "-Werror", + "-E", + errorCode, + ], + { + cwd: fixDir, + }, + ); expect(result.exitCode).toBe(0); const after = readFileSync(path.join(runDir, file), "utf-8"); expect(after).toContain(expected); @@ -66,7 +79,7 @@ describe("check", () => { } test("M0223", async () => { - await checkFix("M0223", "List.empty()", "List.empty()"); + await checkFix("M0223", "identity(1)", "identity(1)"); }); test("M0236", async () => { diff --git a/cli/tests/check/fix/M0223.mo b/cli/tests/check/fix/M0223.mo index 624fc1d0..d11b8ae3 100644 --- a/cli/tests/check/fix/M0223.mo +++ b/cli/tests/check/fix/M0223.mo @@ -1,10 +1,11 @@ // M0223: Redundant type instantiation // The type annotation is not needed when it can be inferred -import List "mo:core/List"; persistent actor { - public func testM0223() : async Bool { - let list : List.List = List.empty(); - list.isEmpty(); + public func testM0223() : async () { + func identity(x : T) : T = x; + let varArray : [var Nat] = [var 1]; + let nat = identity(1); + varArray[0] := nat; }; };