Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report use before definitely defined errors #5207

Closed
falsandtru opened this issue Oct 11, 2015 · 12 comments
Closed

Report use before definitely defined errors #5207

falsandtru opened this issue Oct 11, 2015 · 12 comments
Assignees
Labels
Committed The team has roadmapped this issue Fixed A PR has been merged for this issue Suggestion An idea for TypeScript

Comments

@falsandtru
Copy link
Contributor

This compiled code will throw a ReferenceError exception by native javascript engine if Class syntax is implemented. It means that TS compiler generate the invalid code. Should be disallow the use of a class before definition like a Let and Const Declaration because Class Declaration does not specified as hoisting target by es6 spec.

a; // ts: pass.
   // js: pass.
var a;
b; // ts: compile error
   // js: runtime error(ReferenceError).
let b;
c; // ts: compile error
   // js: runtime error(ReferenceError).
const c = void 0;
f(); // ts: pass.
     // js: pass.
function f(){}

new Foo(); // ts: pass.
           // js: runtime error(ReferenceError)!
           // Class Declaration is not hoistable.
           // should be a compile error before throw a runtime error.
class Foo {}

http://www.ecma-international.org/ecma-262/6.0/#sec-ecmascript-language-statements-and-declarations
http://www.ecma-international.org/ecma-262/6.0/#sec-variable-statement
http://www.2ality.com/2015/02/es6-classes-final.html

I think, this issue is different from #2854 because this issue is based on spec.

@DanielRosenwasser
Copy link
Member

I think @vladima's CFA work might catch this.

@falsandtru
Copy link
Contributor Author

What is CFA? Latest commit does not detect this error.

@basarat
Copy link
Contributor

basarat commented Oct 12, 2015

What is CFA

Code Flow Analysis. Fancy term for code reach-ability checking.

Latest commit does not detect this error.

The work is not on master. Here is one PR : #4788 ... there might be others 🌹

@falsandtru
Copy link
Contributor Author

@basarat thanks! I tested on #4788 but could not detect for now. I think reach-ability check and declaration(and evaluation order) check are different.

@mhegazy
Copy link
Contributor

mhegazy commented Oct 12, 2015

this is not CFA; CFA is detecting unreachable code, e.g. a statement after a return. what this issue is about is Definite Assignment Analysis. you want to know that a variable is definitely assigned to before the first use site.

I think, this issue is different from #2854 because this issue is based on spec.

the JS spec talks about runtime errors. these can not all be detected at design time easily. issue #2854 is not the right issue. i will update this issue title to reflect the request.

@mhegazy mhegazy added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Oct 12, 2015
@mhegazy mhegazy changed the title TS compiler leak a ReferenceError on Class Declaration Report use before definitely defined errors Oct 12, 2015
@RyanCavanaugh RyanCavanaugh added Committed The team has roadmapped this issue and removed In Discussion Not yet reached consensus labels Oct 19, 2015
@RyanCavanaugh
Copy link
Member

@mhegazy will refresh his PR and implement this for extends clauses and import declarations

@mhegazy mhegazy assigned sheetalkamat and unassigned mhegazy Oct 19, 2015
@mhegazy mhegazy added this to the TypeScript 1.8 milestone Oct 19, 2015
@mhegazy
Copy link
Contributor

mhegazy commented Oct 19, 2015

@sheetalkamat please include this in your PR in: #4343

@DanielRosenwasser
Copy link
Member

Just because clicking through several links to find this issue is becoming a massive pain.

This is an issue for TypeScript to:

  • report errors on use before definition
  • report errors on use before declaration
  • report errors on use before define
  • report errors on use before declare
  • report errors on use before initialization

Hopefully that makes this more Google-friendly.

@mhegazy mhegazy modified the milestones: Future, TypeScript 2.1 Sep 21, 2016
Frikki added a commit to SparksNetwork/sparks-frontend that referenced this issue Oct 20, 2016
There is a big chance of TypeScript generating invalid code
with cyclic dependencies.

See microsoft/TypeScript#5207
@pmoleri
Copy link

pmoleri commented Feb 22, 2017

Hi, just to point out that as of v2.1.4 this is still an issue, although this issue has the label commited, which I found a bit confusing.

@mhegazy
Copy link
Contributor

mhegazy commented Feb 22, 2017

Committed: Someone from the TypeScript team will fix this bug or implement this feature

more about labels at: https://github.com/Microsoft/TypeScript/wiki/FAQ#what-do-the-labels-on-these-issues-mean

@pmoleri
Copy link

pmoleri commented Feb 22, 2017

Thanks @mhegazy, that explains my confusion.

@mhegazy
Copy link
Contributor

mhegazy commented Jun 12, 2017

Should be fixed in latest.

@mhegazy mhegazy closed this as completed Jun 12, 2017
@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Jun 12, 2017
@mhegazy mhegazy modified the milestones: TypeScript 2.4, Future Jun 12, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Committed The team has roadmapped this issue Fixed A PR has been merged for this issue Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

8 participants