Skip to content

Conversation

lukaselmer
Copy link
Contributor

@lukaselmer lukaselmer commented Sep 27, 2016

Fixes #10601

  • There is an associated issue that is labelled
    'Bug' or 'Accepting PRs' or is in the Community milestone: see Misleading error TS2410 #10601
  • Code is up-to-date with the master branch
  • You've signed the CLA
  • There are new or updated unit tests validating the change
  • You've successfully run jake runtests locally: No, tslint did not run when I checked out the project. The tests run though.

@msftclas
Copy link

Hi @lukaselmer, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

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

One question for investigation and one request for rewording the error message.

@@ -1279,7 +1279,7 @@
"category": "Error",
"code": 2409
},
"All symbols within a 'with' block will be resolved to 'any'.": {
"Unsupported 'with' statement, all symbols within a 'with' block will be resolved to 'any'.": {
Copy link
Member

Choose a reason for hiding this comment

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

I would word the error as "The 'with' statement is not supported. All symbols in a 'with' block will have type 'any'."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can do that.

if (!hasParseDiagnostics(sourceFile)) {
const start = getSpanOfTokenAtPosition(sourceFile, node.pos).start;
const end = node.statement.pos;
grammarErrorAtPos(sourceFile, start, end - start, Diagnostics.Unsupported_with_statement_all_symbols_within_a_with_block_will_be_resolved_to_any);
Copy link
Member

Choose a reason for hiding this comment

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

How bad is the error span if you just say error(node, ...) ? It would save a lot of fiddly code. But I suspect that it underlines the entire with block.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it underlines the whole with block. Therefore I would still go with this solution.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good.

@sandersn sandersn merged commit 6efa8bf into microsoft:master Sep 29, 2016
@lukaselmer lukaselmer deleted the fix10601 branch September 29, 2016 17:34
@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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants