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

Increase size of span for unused declaration #22388

Merged
1 commit merged into from
Mar 7, 2018
Merged

Increase size of span for unused declaration #22388

1 commit merged into from
Mar 7, 2018

Conversation

ghost
Copy link

@ghost ghost commented Mar 7, 2018

@ghost ghost requested review from amcasey, mhegazy and sheetalkamat March 7, 2018 21:38
@ghost ghost merged commit 0be9ee2 into master Mar 7, 2018
@ghost ghost deleted the unused_bigger_span branch March 7, 2018 22:14
Copy link
Contributor

@mihailik mihailik left a comment

Choose a reason for hiding this comment

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

More tests needed, there are a few use cases where outcome is ambiguous without a test pegging to specific span.

@@ -1,10 +1,10 @@
tests/cases/compiler/unusedClassesinModule1.ts(2,11): error TS6133: 'Calculator' is declared but its value is never read.
tests/cases/compiler/unusedClassesinModule1.ts(2,5): error TS6133: 'Calculator' is declared but its value is never read.


==== tests/cases/compiler/unusedClassesinModule1.ts (1 errors) ====
module A {
class Calculator {
Copy link
Contributor

Choose a reason for hiding this comment

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

What's going to happen for generic class?

Calculator<T>
Calculator<T where T extends K>

tests/cases/compiler/noUnusedLocals_selfReference.ts(12,1): error TS6133: 'E' is declared but its value is never read.
tests/cases/compiler/noUnusedLocals_selfReference.ts(13,1): error TS6133: 'I' is declared but its value is never read.
tests/cases/compiler/noUnusedLocals_selfReference.ts(14,1): error TS6133: 'T' is declared but its value is never read.
tests/cases/compiler/noUnusedLocals_selfReference.ts(15,1): error TS6133: 'N' is declared but its value is never read.
tests/cases/compiler/noUnusedLocals_selfReference.ts(22,19): error TS6133: 'm' is declared but its value is never read.


==== tests/cases/compiler/noUnusedLocals_selfReference.ts (8 errors) ====
export {}; // Make this a module scope, so these are local variables.

function f() {
Copy link
Contributor

Choose a reason for hiding this comment

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

What about generic functions?

function f<T>() {
function f<T where T extends K> () {

Also what about async functions?

async function f() {

Also what about class methods?

private f() {

tests/cases/compiler/noUnusedLocals_selfReference_skipsBlockLocations.ts(12,22): error TS6133: 'h' is declared but its value is never read.
tests/cases/compiler/noUnusedLocals_selfReference_skipsBlockLocations.ts(2,5): error TS6133: 'f' is declared but its value is never read.
tests/cases/compiler/noUnusedLocals_selfReference_skipsBlockLocations.ts(8,13): error TS6133: 'g' is declared but its value is never read.
tests/cases/compiler/noUnusedLocals_selfReference_skipsBlockLocations.ts(12,13): error TS6133: 'h' is declared but its value is never read.


==== tests/cases/compiler/noUnusedLocals_selfReference_skipsBlockLocations.ts (3 errors) ====
namespace n {
function f() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Comments or extensive whitespace/newlines between function and f — will those be included in the span, or break off the function part out of the span?

@@ -1,10 +1,10 @@
tests/cases/compiler/unusedNamespaceInNamespace.ts(2,15): error TS6133: 'B' is declared but its value is never read.
tests/cases/compiler/unusedNamespaceInNamespace.ts(2,5): error TS6133: 'B' is declared but its value is never read.


==== tests/cases/compiler/unusedNamespaceInNamespace.ts (1 errors) ====
namespace A {
namespace B { }
Copy link
Contributor

Choose a reason for hiding this comment

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

Worth adding enum, var/let/const tests too.
Anything else?

ghost pushed a commit that referenced this pull request Jun 1, 2018
ghost pushed a commit that referenced this pull request Jun 1, 2018
ghost pushed a commit that referenced this pull request Jun 1, 2018
@microsoft microsoft locked and limited conversation to collaborators Jul 25, 2018
This pull request was closed.
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