GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Simple control flow analysis by vladima · Pull Request #1287 · microsoft/TypeScript

Conversation

@vladima

@vladima Vladimir Matveev (vladima) commented Nov 27, 2014

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Vladimir Matveev (vladima) added 2 commits November 7, 2014 22:21
@vladima
initial rev of controlflow analysis
3ec5eb1
@vladima
merge with master
d0da055
DanielRosenwasser
DanielRosenwasser reviewed Nov 27, 2014
View reviewed changes
Comment thread src/compiler/checker.ts

@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) Nov 27, 2014

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@dekajp dekajp (dekajp) mentioned this pull request Nov 30, 2014
Switch case fallthrough #393
Closed
DanielRosenwasser
DanielRosenwasser reviewed Nov 30, 2014
View reviewed changes
Comment thread src/compiler/controlflow.ts

@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) Nov 30, 2014

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@vladima
addressed CR feedback, exit early after reporting 'unreachable statem…
1d6bd0d 
…ent error'
DanielRosenwasser
DanielRosenwasser reviewed Dec 1, 2014
View reviewed changes
Comment thread src/compiler/controlflow.ts

@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) Dec 1, 2014

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Vladimir Matveev (vladima) added 2 commits December 1, 2014 10:38
@vladima
merge with master
80cc8bb
@vladima
removed commented code
147959f
@JsonFreeman

Jason Freeman (JsonFreeman) commented Dec 1, 2014

Copy link
Copy Markdown
Contributor

👍

Awesome job!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Vladimir Matveev (vladima) added 4 commits December 1, 2014 13:50
@vladima
fix post finally state
20d0cd9
@vladima
move check of else statement into conditional
363a517
@vladima
change category of cf related notification to 'warning'
15e630c
@vladima
fix final state in if statement
68b135a
@vladima Vladimir Matveev (vladima) mentioned this pull request Jan 21, 2015
Type narrowing is not allowed if variable is reassigned after the use site inside if block #1764
Closed
@DanielRosenwasser

Daniel Rosenwasser (DanielRosenwasser) commented Jan 22, 2015

Copy link
Copy Markdown
Member

We should put this back on the radar again.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@NoelAbrahams

Noel Abrahams (NoelAbrahams) commented Jan 22, 2015

Copy link
Copy Markdown

We should put this back on the radar again.

👍
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@CyrusNajmabadi Cyrus Najmabadi (CyrusNajmabadi) mentioned this pull request Mar 1, 2015
Suggestion: errors based on control flow analysis #2176
Closed
@awerlang

André Werlang (awerlang) commented Mar 19, 2015

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@CyrusNajmabadi Cyrus Najmabadi (CyrusNajmabadi) mentioned this pull request Apr 10, 2015
If the expression after the return statement is on a new line, the generated code is incorrect #2704
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@vladima @JsonFreeman @DanielRosenwasser @NoelAbrahams @awerlang @CyrusNajmabadi
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.