Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions localtypings/pxtarget.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[];
Expand Down
24 changes: 16 additions & 8 deletions pxtcompiler/emitter/driver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -237,16 +245,16 @@ 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);
Expand All @@ -256,12 +264,12 @@ namespace ts.pxtc {

// If we didn't have any 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()
Expand All @@ -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) {
Expand Down
21 changes: 19 additions & 2 deletions pxtcompiler/emitter/emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -4241,8 +4241,25 @@ ${lbl}: .short 0xffff
return v
}

function maybeWarnOnBareFunctionReference(node: Expression) {
if (!opts.enhancedErrors)
return

if (!node || node.kind != SK.Identifier)
Comment thread
jwunderl marked this conversation as resolved.
Outdated
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 emitExprAsStmt(node: Expression) {
if (isNoopExpr(node)) return
if (isNoopExpr(node)) {
maybeWarnOnBareFunctionReference(node)
return
}
emitBrk(node)
let v = emitIgnored(node)
proc.emitExpr(v)
Expand Down
7 changes: 5 additions & 2 deletions pxtcompiler/emitter/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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)))
Expand Down
16 changes: 16 additions & 0 deletions tests/errors-test/cases/bare-function-reference.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// enhancedErrors

function doSomething() {
}

doSomething // TS9284

let handler = doSomething
handler // TS9284

function runCallback(cb: () => void) {
cb()
}

runCallback(doSomething)
let stored = doSomething
12 changes: 12 additions & 0 deletions tests/errors-test/errorrunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions webapp/src/compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -572,6 +572,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 });

Expand Down
7 changes: 6 additions & 1 deletion webapp/src/monaco.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1737,9 +1737,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
Comment thread
jwunderl marked this conversation as resolved.
Outdated
? monaco.MarkerSeverity.Error
: d.category == ts.pxtc.DiagnosticCategory.Warning
Comment thread
jwunderl marked this conversation as resolved.
Outdated
? 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,
Expand Down
Loading