Skip to content

fix: matchedText module resolution incorrect length#21647

Merged
3 commits merged into
microsoft:masterfrom
alan-agius4:feature/path-resolution
Feb 7, 2018
Merged

fix: matchedText module resolution incorrect length#21647
3 commits merged into
microsoft:masterfrom
alan-agius4:feature/path-resolution

Conversation

@alan-agius4
Copy link
Copy Markdown
Contributor

@alan-agius4 alan-agius4 commented Feb 5, 2018

matchedText was not properly extract the correct parts from the candidate.

Fixes #21636

@alan-agius4 alan-agius4 closed this Feb 5, 2018
@alan-agius4 alan-agius4 reopened this Feb 5, 2018
@alan-agius4 alan-agius4 force-pushed the feature/path-resolution branch from 2b4156c to 3a66b8e Compare February 5, 2018 22:22
@alan-agius4 alan-agius4 force-pushed the feature/path-resolution branch from 3a66b8e to 18847f9 Compare February 6, 2018 06:48
@alan-agius4
Copy link
Copy Markdown
Contributor Author

@Andy-MS can you take a look at this please? Thanks


// @filename: c:/root/index.ts
import {x} from "@speedy/folder1/testing"
declare function use(a: any): void;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Are these two lines necessary? --noUnusedLocals is disabled by default.

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.

In that case it's safe to remove

Comment thread src/compiler/core.ts Outdated
Debug.assert(isPatternMatch(pattern, candidate));
return candidate.substr(pattern.prefix.length, candidate.length - pattern.suffix.length);
const matchLength = candidate.length - pattern.suffix.length - pattern.prefix.length;
return candidate.substr(pattern.prefix.length, matchLength);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Could use slice which takes start and end and is more common in the rest of the codebase. substr leads to nothing but problems (#20578).

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 think you meant substring not slice slice but i get your point :)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks like we use them both about as often. According to this slice and substring are basically equivalent with the exception of error handling -- if you avoid negative numbers, NaN, and start > end, they're the same.

@alan-agius4 alan-agius4 force-pushed the feature/path-resolution branch from 794c673 to d59a360 Compare February 7, 2018 20:17
@alan-agius4 alan-agius4 force-pushed the feature/path-resolution branch from d59a360 to d01d068 Compare February 7, 2018 20:38
@alan-agius4
Copy link
Copy Markdown
Contributor Author

@Andy-MS thanks for the review. Updated as suggested!

@ghost ghost merged commit 3893ed4 into microsoft:master Feb 7, 2018
@alan-agius4 alan-agius4 deleted the feature/path-resolution branch February 7, 2018 20:55
@microsoft microsoft locked and limited conversation to collaborators Jul 3, 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.

1 participant