FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Increase size of span for unused declaration · Pull Request #22388 · microsoft/TypeScript · GitHub

Increase size of span for unused declaration - #22388

Merged
1 commit merged into
masterfrom
unused_bigger_span
Mar 7, 2018
Merged

Increase size of span for unused declaration#22388
1 commit merged into
masterfrom
unused_bigger_span

Conversation

Copy link
Copy Markdown

ghost merged commit 0be9ee2 into master Mar 7, 2018
ghost deleted the unused_bigger_span branch March 7, 2018 22:14

mihailik left a comment

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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


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

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

What's going to happen for generic class?

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

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

function f() {

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 (3 errors) ====
namespace n {
function f() {

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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?


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

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

This pull request was closed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL