Skip to content

Simple control flow analysis - #1287

Closed
Vladimir Matveev (vladima) wants to merge 9 commits into
masterfrom
cfa
Closed

Simple control flow analysis #1287
Vladimir Matveev (vladima) wants to merge 9 commits into
masterfrom
cfa

Conversation

@vladima

Copy link
Copy Markdown
Contributor

This PR adds a basic control flow analysis to typescript compiler. Now it can detect unreachable code, implicit returns of undefined in functions, unused labels, fall- through between cases in switch statements. Some of these checks can be used in #393 or #274.

cfa

Disclaimer: This PR is not intended to be checked in as-is. What I rather want is to use it as a starting point to discuss:

  • difference between warnings and errors. now all semantic errors that are produced by the compler can be treated as warnings - they do not block emit. However visually they are always presented as errors which might be confusing, I.e. I would expect message about unreachable code to be more like recommendation (presented using green squigglies or shadowed code) instead of being aggressively marked as error
  • error suppression story - global and local
Comment thread src/compiler/checker.ts

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider controlFlow.ts, but I like this too.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leave a comment to explain that name is optional for the case of implicit labels.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider c.statements.length > 0

@JsonFreeman

Copy link
Copy Markdown
Contributor

👍

Awesome job!

@DanielRosenwasser

Copy link
Copy Markdown
Member

We should put this back on the radar again.

@NoelAbrahams

Copy link
Copy Markdown

We should put this back on the radar again.

👍

@awerlang

Copy link
Copy Markdown

IMO, it overlaps with TSLint.

Regarding this, I'm concerned with the fact that I cannot use more mature JSHint or ESLint. Perhaps running them on the output (haven't tried that though).
As a TS user, I would prefer that it becomes a full superset of ES6, so I could have back other linters in my workflow.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

6 participants