warn on function ref without invokation as statement

localtypings/pxtarget.d.ts

@@ -1132,6 +1132,7 @@ declare namespace ts.pxtc {
         fileSystem: pxt.Map<string>;
         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[];

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,31 +245,31 @@ 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)
         }
 
         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);
             }
             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) {

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);
         (<any>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)

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,15 +1456,17 @@ 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;
                 ts2asm = compile(host.opts, service);
                 res = {
                     sourceMap: res.sourceMap,
                     ...ts2asm,
+                    diagnostics: conversionDiagnostics.concat(ts2asm.diagnostics),
                 }
             }
             if (res.diagnostics.every(d => !isPxtModulesFilename(d.fileName)))

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

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) {

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;
+                }
             }
         }
     }

webapp/src/compiler.ts

@@ -174,6 +174,7 @@ export function compileAsync(options: CompileOptions = {}): Promise<pxtc.Compile
             }
             opts.computeUsedSymbols = true;
             opts.computeUsedParts = true;
+            opts.enhancedErrors = true;
             if (options.forceEmit)
                 opts.forceEmit = true;
             if (/test=1/i.test(window.location.href))
@@ -572,6 +573,7 @@ export async function typecheckAsync(): Promise<pxtc.CompileResult | null> {
 
             const opts = await pkg.mainPkg.getCompileOptionsAsync();
             opts.testMode = true; // show errors in all top-level code
+            opts.enhancedErrors = true;
 
             await workerOpAsync("setOptions", { options: opts });
 

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<ErrorListItemProps, ErrorListItemSta
         const hasStack = !!error.stackFrames && error.stackFrames.length > 0;
         const topRowClass = hasStack ? "exceptionMessage" : classList("item", className);
         const errorCounter = (errorGroup.count <= 1) ? null : <div className="ui gray circular label countBubble">{errorGroup.count}</div>;
-
-        const itemHeaderRow = isInteractive ? (
+        const errorMessage = <>
+            <span className="errorListItemText">{error.message}</span>
+            {errorCounter}
+        </>;
+        const actionButtons = error.actions?.length ? (
+            <div className="errorListItemActions">
+                {error.actions.map((action, index) => (
+                    <Button
+                        key={index}
+                        className="secondary errorListAction"
+                        onClick={action.onClick}
+                        title={action.title || action.label}
+                        ariaLabel={action.ariaLabel || action.title || action.label}
+                        label={action.label}
+                    />
+                ))}
+            </div>
+        ) : undefined;
+
+        const itemHeaderRow = actionButtons ? (
+            <div className={classList(topRowClass, "errorListItemRow")}>
+                {isInteractive ? (
+                    <Button className="errorListItemMessage"
+                        onClick={error.onClick}
+                        title={lf("Go to error: {0}", error.message)}
+                        ariaLabel={lf("Go to error: {0}", error.message)}>
+                        {errorMessage}
+                    </Button>
+                ) : (
+                    <div className="errorListItemMessage" aria-label={error.message} tabIndex={0}>
+                        {errorMessage}
+                    </div>
+                )}
+                {actionButtons}
+            </div>
+        ) : isInteractive ? (
             <Button className={topRowClass}
                 onClick={error.onClick}
                 title={lf("Go to error: {0}", error.message)}
-                aria-label={lf("Go to error: {0}", error.message)}>
-                <div>
-                    <span>{error.message}</span>
-                    {errorCounter}
-                </div>
+                ariaLabel={lf("Go to error: {0}", error.message)}>
+                <div>{errorMessage}</div>
             </Button>
         ) : (
              <div className={topRowClass} aria-label={error.message} tabIndex={0}>
-                <span>{error.message}</span>
-                {errorCounter}
+                {errorMessage}
             </div>
         );