diff --git a/localtypings/pxtarget.d.ts b/localtypings/pxtarget.d.ts index f3cbe35a88b8..f4e810ec2ecd 100644 --- a/localtypings/pxtarget.d.ts +++ b/localtypings/pxtarget.d.ts @@ -1132,6 +1132,7 @@ declare namespace ts.pxtc { fileSystem: pxt.Map; target: CompileTarget; testMode?: boolean; + enhancedErrors?: boolean; // enable extra editor-only diagnostics sourceFiles?: string[]; // list of file names sourceTexts?: string[]; // list of file text content (TS string) generatedFiles?: string[]; diff --git a/pxtcompiler/emitter/driver.ts b/pxtcompiler/emitter/driver.ts index 849dfeb8d53a..61d00083504f 100644 --- a/pxtcompiler/emitter/driver.ts +++ b/pxtcompiler/emitter/driver.ts @@ -92,6 +92,14 @@ namespace ts.pxtc { }) } + export function hasErrorDiagnostics(diags: ReadonlyArray<{ category: DiagnosticCategory }>) { + return diags.some(d => d.category == DiagnosticCategory.Error) + } + + export function hasBlockingDiagnostics(opts: CompileOptions, diags: ReadonlyArray<{ category: DiagnosticCategory }>) { + return opts.enhancedErrors ? hasErrorDiagnostics(diags) : diags.length > 0 + } + export function py2tsIfNecessary(opts: CompileOptions): transpile.TranspileResult | undefined { if (opts.target.preferredEditor == pxt.PYTHON_PROJECT_NAME) { let res = pxtc.transpile.pyToTs(opts) @@ -237,7 +245,7 @@ namespace ts.pxtc { program = service.getProgram() } else { runConversionsAndStoreResults(opts, res) - if (res.diagnostics.length > 0) + if (hasBlockingDiagnostics(opts, res.diagnostics)) return res; program = buildProgram(opts, res) } @@ -245,8 +253,8 @@ namespace ts.pxtc { const entryPoint = opts.sourceFiles.filter(f => U.endsWith(f, ".ts")).pop().replace(/.*\//, "") // First get and report any syntactic errors. - res.diagnostics = patchUpDiagnostics(program.getSyntacticDiagnostics(), opts.ignoreFileResolutionErrors); - if (res.diagnostics.length > 0) { + res.diagnostics = res.diagnostics.concat(patchUpDiagnostics(program.getSyntacticDiagnostics(), opts.ignoreFileResolutionErrors)); + if (hasBlockingDiagnostics(opts, res.diagnostics)) { if (opts.forceEmit) { pxt.debug('syntactic errors, forcing emit') compileBinary(program, opts, res, entryPoint); @@ -254,14 +262,14 @@ namespace ts.pxtc { return res; } - // If we didn't have any syntactic errors, then also try getting the global and + // If we didn't have any blocking syntactic errors, then also try getting the global and // semantic errors. - res.diagnostics = patchUpDiagnostics(program.getOptionsDiagnostics().concat(Util.toArray(program.getGlobalDiagnostics())), opts.ignoreFileResolutionErrors); + res.diagnostics = res.diagnostics.concat(patchUpDiagnostics(program.getOptionsDiagnostics().concat(Util.toArray(program.getGlobalDiagnostics())), opts.ignoreFileResolutionErrors)); const semStart = U.cpuUs() - if (res.diagnostics.length == 0) { - res.diagnostics = patchUpDiagnostics(program.getSemanticDiagnostics(), opts.ignoreFileResolutionErrors); + if (!hasBlockingDiagnostics(opts, res.diagnostics)) { + res.diagnostics = res.diagnostics.concat(patchUpDiagnostics(program.getSemanticDiagnostics(), opts.ignoreFileResolutionErrors)); } const emitStart = U.cpuUs() @@ -273,13 +281,13 @@ namespace ts.pxtc { res.ast = program } - if (opts.ast || opts.forceEmit || res.diagnostics.length == 0) { + if (opts.ast || opts.forceEmit || !hasBlockingDiagnostics(opts, res.diagnostics)) { const binOutput = compileBinary(program, opts, res, entryPoint); res.times["compilebinary"] = U.cpuUs() - emitStart res.diagnostics = res.diagnostics.concat(patchUpDiagnostics(binOutput.diagnostics)) } - if (res.diagnostics.length == 0) + if (!hasBlockingDiagnostics(opts, res.diagnostics)) res.success = true for (let f of opts.sourceFiles) { diff --git a/pxtcompiler/emitter/emitter.ts b/pxtcompiler/emitter/emitter.ts index e75d88c798f8..83fecb00a587 100644 --- a/pxtcompiler/emitter/emitter.ts +++ b/pxtcompiler/emitter/emitter.ts @@ -247,7 +247,7 @@ namespace ts.pxtc { pxt.log(stringKind(n)) } - // next free error 9284 + // next free error 9285 function userError(code: number, msg: string, secondary = false): Error { let e = new Error(msg); (e).ksEmitterUserError = true; @@ -4241,8 +4241,41 @@ ${lbl}: .short 0xffff return v } + function maybeWarnOnBareFunctionReference(node: Expression) { + if (!opts.enhancedErrors) + return + + if (!node || node.kind !== SK.Identifier) + return + + if (hasPrecedingTsIgnoreComment(node)) + return + + const exprType = typeOf(node) + const signatures = exprType && !(exprType.flags & TypeFlags.Any) + && checker.getSignaturesOfType(exprType, SignatureKind.Call) + if (signatures && signatures.length) + warning(node, 9284, lf("Function reference used as a statement; did you mean to call it with '()'?")) + } + + function hasPrecedingTsIgnoreComment(node: Node) { + const src = node.getSourceFile() + const line = ts.getLineAndCharacterOfPosition(src, node.getStart(src)).line + if (line <= 0) + return false + + const lineStarts = src.getLineStarts() + const previousLineStart = lineStarts[line - 1] + const previousLineEnd = lineStarts[line] + const previousLine = src.text.slice(previousLineStart, previousLineEnd) + return /^\s*\/\/\s*@ts-ignore\b/.test(previousLine) + } + function emitExprAsStmt(node: Expression) { - if (isNoopExpr(node)) return + if (isNoopExpr(node)) { + maybeWarnOnBareFunctionReference(node) + return + } emitBrk(node) let v = emitIgnored(node) proc.emitExpr(v) diff --git a/pxtcompiler/emitter/service.ts b/pxtcompiler/emitter/service.ts index def9229882f6..b27bc322f192 100644 --- a/pxtcompiler/emitter/service.ts +++ b/pxtcompiler/emitter/service.ts @@ -1436,7 +1436,8 @@ namespace ts.pxtc.service { for (let k of Object.keys(newFS)) host.setFile(k, newFS[k]) // update version numbers res.fileSystem = U.flatClone(newFS) - if (res.diagnostics.length == 0) { + if (!hasBlockingDiagnostics(host.opts, res.diagnostics)) { + const conversionDiagnostics = res.diagnostics host.opts.skipPxtModulesEmit = false host.opts.skipPxtModulesTSC = false const currKey = host.opts.target.isNative ? "native" : "js" @@ -1455,8 +1456,9 @@ namespace ts.pxtc.service { sourceMap: res.sourceMap, fileSystem: res.fileSystem, ...ts2asm, + diagnostics: conversionDiagnostics.concat(ts2asm.diagnostics), } - if (res.needsFullRecompile || ((!res.success || res.diagnostics.length) && host.opts.clearIncrBuildAndRetryOnError)) { + if (res.needsFullRecompile || ((!res.success || hasBlockingDiagnostics(host.opts, res.diagnostics)) && host.opts.clearIncrBuildAndRetryOnError)) { pxt.debug("triggering full recompile") pxt.tickEvent("compile.fullrecompile") host.opts.skipPxtModulesEmit = false; @@ -1464,6 +1466,7 @@ namespace ts.pxtc.service { res = { sourceMap: res.sourceMap, ...ts2asm, + diagnostics: conversionDiagnostics.concat(ts2asm.diagnostics), } } if (res.diagnostics.every(d => !isPxtModulesFilename(d.fileName))) diff --git a/tests/errors-test/cases/bare-function-reference.ts b/tests/errors-test/cases/bare-function-reference.ts new file mode 100644 index 000000000000..e20f2c6b727a --- /dev/null +++ b/tests/errors-test/cases/bare-function-reference.ts @@ -0,0 +1,19 @@ +// enhancedErrors + +function doSomething() { +} + +doSomething // TS9284 + +let handler = doSomething +handler // TS9284 + +// @ts-ignore +doSomething + +function runCallback(cb: () => void) { + cb() +} + +runCallback(doSomething) +let stored = doSomething \ No newline at end of file diff --git a/tests/errors-test/errorrunner.ts b/tests/errors-test/errorrunner.ts index 4cb51285099c..ba50de6e1ca9 100644 --- a/tests/errors-test/errorrunner.ts +++ b/tests/errors-test/errorrunner.ts @@ -74,8 +74,20 @@ async function stsErrorTestAsync(filename: string) { const opts = await pkg.getCompileOptionsAsync(target); opts.testMode = true; + const hasEnhancedErrors = /^\/\/\s*enhancedErrors\b/.test(text); + if (hasEnhancedErrors) { + const plainOpts = await pkg.getCompileOptionsAsync(target); + plainOpts.testMode = true; + const plainRes = pxtc.compile(plainOpts); + chai.assert(!plainRes.diagnostics.some(d => d.code == 9284), `${basename}: enhanced errors should require opt-in`); + } + + opts.enhancedErrors = hasEnhancedErrors; const res = pxtc.compile(opts); checkDiagnostics(res.diagnostics, basename, text); + if (!res.diagnostics.some(d => d.category == pxtc.DiagnosticCategory.Error)) { + chai.assert(res.success, `${basename}: warning-only diagnostics should not fail compile`); + } } async function pyErrorTestAsync(filename: string) { diff --git a/theme/errorList.less b/theme/errorList.less index ad96b0d9d5ff..b04b3eccfbcb 100644 --- a/theme/errorList.less +++ b/theme/errorList.less @@ -152,6 +152,44 @@ &.stackframe { padding-left: 2em; } + + &.errorListItemRow { + display: flex; + align-items: center; + gap: 0.5rem; + } + + .errorListItemMessage { + flex: 1 1 auto; + min-width: 0; + margin: 0; + padding: 0; + background: none; + color: inherit; + line-height: inherit; + text-align: start; + + &:hover { + filter: none; + } + } + + .errorListItemText { + overflow-wrap: anywhere; + } + + .errorListItemActions { + flex: 0 0 auto; + display: flex; + align-items: center; + } + + .errorListAction { + margin: 0; + padding: 0.7rem; + line-height: 0.8rem; + white-space: nowrap; + } } } } diff --git a/webapp/src/compiler.ts b/webapp/src/compiler.ts index 218814ed1a47..a9bf666f5344 100644 --- a/webapp/src/compiler.ts +++ b/webapp/src/compiler.ts @@ -174,6 +174,7 @@ export function compileAsync(options: CompileOptions = {}): Promise { const opts = await pkg.mainPkg.getCompileOptionsAsync(); opts.testMode = true; // show errors in all top-level code + opts.enhancedErrors = true; await workerOpAsync("setOptions", { options: opts }); diff --git a/webapp/src/errorList.tsx b/webapp/src/errorList.tsx index 55cb7c4192ba..0eb1b26cf838 100644 --- a/webapp/src/errorList.tsx +++ b/webapp/src/errorList.tsx @@ -26,12 +26,20 @@ export type StackFrameDisplayInfo = { onClick?: () => void; }; +export interface ErrorDisplayAction { + label: string; + title?: string; + ariaLabel?: string; + onClick: () => void; +} + export type ErrorDisplayInfo = { message: string; stackFrames?: StackFrameDisplayInfo[]; metadata?: ErrorMetadata; preventsRunning?: boolean; onClick?: () => void; + actions?: ErrorDisplayAction[]; }; export interface ErrorListProps { @@ -230,21 +238,51 @@ class ErrorListItem extends React.Component 0; const topRowClass = hasStack ? "exceptionMessage" : classList("item", className); const errorCounter = (errorGroup.count <= 1) ? null :
{errorGroup.count}
; - - const itemHeaderRow = isInteractive ? ( + const errorMessage = <> + {error.message} + {errorCounter} + ; + const actionButtons = error.actions?.length ? ( +
+ {error.actions.map((action, index) => ( +
+ ) : undefined; + + const itemHeaderRow = actionButtons ? ( +
+ {isInteractive ? ( + + ) : ( +
+ {errorMessage} +
+ )} + {actionButtons} +
+ ) : isInteractive ? ( ) : (
- {error.message} - {errorCounter} + {errorMessage}
); diff --git a/webapp/src/monaco.tsx b/webapp/src/monaco.tsx index adb36cec8218..e4a2a05a5942 100644 --- a/webapp/src/monaco.tsx +++ b/webapp/src/monaco.tsx @@ -36,6 +36,9 @@ import { AIErrorExplanationText } from "./components/AIErrorExplanationText"; const MIN_EDITOR_FONT_SIZE = 10 const MAX_EDITOR_FONT_SIZE = 40 +const SUPPRESSIBLE_ENHANCED_DIAGNOSTICS = [9284] +const TS_IGNORE_COMMENT = "// @ts-ignore" +const TS_IGNORE_COMMENT_REGEX = /^\s*\/\/\s*@ts-ignore\b/ interface TranspileResult { success: boolean; @@ -742,13 +745,48 @@ export class Editor extends toolboxeditor.ToolboxEditor { private getDisplayInfoForError(error: pxtc.KsDiagnostic): ErrorDisplayInfo { const message = lf("Line {0}: {1}", error.endLine ? error.endLine + 1 : error.line + 1, error.messageText); + const actions = error.category == ts.pxtc.DiagnosticCategory.Warning && SUPPRESSIBLE_ENHANCED_DIAGNOSTICS.indexOf(error.code) !== -1 ? [{ + label: lf("Ignore"), + title: lf("Ignore this warning"), + ariaLabel: lf("Ignore this warning"), + onClick: () => this.insertTsIgnoreForDiagnostic(error) + }] : undefined; return { message, onClick: () => this.goToError(error), - preventsRunning: true + preventsRunning: error.category == ts.pxtc.DiagnosticCategory.Error, + actions }; } + private insertTsIgnoreForDiagnostic(error: pxtc.KsDiagnostic) { + if (error.line === undefined) + return; + + const model = this.editor.getModel(); + const lineNumber = error.line + 1; + if (lineNumber > 1 && TS_IGNORE_COMMENT_REGEX.test(model.getLineContent(lineNumber - 1))) { + this.goToError(error); + return; + } + + const lineContent = model.getLineContent(lineNumber); + const indentation = /^\s*/.exec(lineContent)[0]; + + this.editor.pushUndoStop(); + this.editor.executeEdits("ignoreDiagnostic", [{ + range: new monaco.Range(lineNumber, 1, lineNumber, 1), + text: `${indentation}${TS_IGNORE_COMMENT}${model.getEOL()}`, + forceMoveMarkers: true + }]); + this.beforeCompile(); + this.editor.pushUndoStop(); + + this.editor.revealLineInCenter(lineNumber); + this.editor.setPosition({ lineNumber: lineNumber + 1, column: (error.column || 0) + 1 }); + this.editor.focus(); + } + /** * Provides a user-facing explanation of the errors in the error list. */ @@ -1737,9 +1775,14 @@ export class Editor extends toolboxeditor.ToolboxEditor { if (file && file.diagnostics) { const model = monaco.editor.getModel(monaco.Uri.parse(`pkg:${file.getName()}`)) for (let d of file.diagnostics) { + const severity = d.category === ts.pxtc.DiagnosticCategory.Error + ? monaco.MarkerSeverity.Error + : d.category === ts.pxtc.DiagnosticCategory.Warning + ? monaco.MarkerSeverity.Warning + : monaco.MarkerSeverity.Info; const addErrorMessage = (message: string) => { monacoErrors.push({ - severity: monaco.MarkerSeverity.Error, + severity, message: message, startLineNumber: d.line + 1, startColumn: d.column + 1,