Skip to content

fix #15155: improve namespace module error message#15176

Merged
sandersn merged 3 commits into
microsoft:masterfrom
HerringtonDarkholme:namespace-error
Apr 14, 2017
Merged

fix #15155: improve namespace module error message#15176
sandersn merged 3 commits into
microsoft:masterfrom
HerringtonDarkholme:namespace-error

Conversation

@HerringtonDarkholme
Copy link
Copy Markdown
Contributor

@HerringtonDarkholme HerringtonDarkholme commented Apr 13, 2017

Fixes #15155

Copy link
Copy Markdown
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.

I would remove the new test since it's redundant. After that, 👍

Comment thread tests/cases/compiler/namespaceModule.ts Outdated
@@ -0,0 +1,9 @@
namespace z {}
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.

Call me crazy, but I don't actually think we need a new test for this change. 🐱 memberScope.errors.txt and importDeclWithDeclareModifier.errors.txt, for example, show exactly the improved error message that is intended.

var a: A; // error
~
!!! error TS2304: Cannot find name 'A'.
!!! error TS2708: Cannot use namespace 'A' as a value here because it has no value export.
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.

The more specific error here is now wrong instead of being vague.

I think I like 'wrong' over 'vague' though.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have polished the error message. Now it will tell if namespace is used as value or type.

Comment thread src/compiler/diagnosticMessages.json Outdated
"category": "Error",
"code": 2707
},
"Cannot use namespace '{0}' as a value here because it has no value export.": {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cannot use namespace '{0}' as a value.

@sandersn
Copy link
Copy Markdown
Member

Thanks for the further improvement in the error message. The final change is very nice.

@sandersn sandersn merged commit 7d1966b into microsoft:master Apr 14, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 21, 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.

4 participants