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

String.localeCompare should allow undefined locale #10618

Closed
jeffreymorlan opened this issue Aug 31, 2016 · 2 comments
Closed

String.localeCompare should allow undefined locale #10618

jeffreymorlan opened this issue Aug 31, 2016 · 2 comments
Labels
Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Fixed A PR has been merged for this issue Help Wanted You can do this

Comments

@jeffreymorlan
Copy link
Contributor

It's legal to pass undefined to String.localeCompare to use the default locale:

function caseInsensitiveCompare(a: string, b: string) {
    return a.localeCompare(b, undefined, { sensitivity: "base" });
}

In TS 2.0 with --structNullChecks enabled, this correct code is disallowed: "Argument of type 'undefined' is not assignable to parameter of type 'string'."

@mhegazy
Copy link
Contributor

mhegazy commented Aug 31, 2016

A PR would be appreciated.

@mhegazy mhegazy added Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Help Wanted You can do this Bug A bug in TypeScript labels Aug 31, 2016
@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Sep 1, 2016
@mhegazy mhegazy modified the milestone: TypeScript 2.1 Sep 1, 2016
@mhegazy mhegazy removed the Fixed A PR has been merged for this issue label Sep 1, 2016
@mhegazy mhegazy reopened this Sep 1, 2016
jeffreymorlan added a commit to jeffreymorlan/TypeScript that referenced this issue Sep 10, 2016
1. Make String.prototype.localeCompare's `locales` parameter optional,
   so `undefined` is allowed (see issue microsoft#10618).
2. Declare the `locales` parameter as a `string | string[]` union
   instead of using overloads. Having separate overloads for `string`
   and `string[]` unnecessarily prevents passing a `string | string[]`.
   (These overloads predate the introduction of union types.)
jeffreymorlan added a commit to jeffreymorlan/TypeScript that referenced this issue Sep 10, 2016
1. Make String.prototype.localeCompare's `locales` parameter optional,
   so `undefined` is allowed.
2. Declare the `locales` parameter as a `string | string[]` union
   instead of using overloads. Having separate overloads for `string`
   and `string[]` unnecessarily prevents passing a `string | string[]`.
   (These overloads predate the introduction of union types.)
@jeffreymorlan
Copy link
Contributor Author

Another problem with localeCompare's and some other internationalization functions' declarations, is that although string and string[] are both legal ways to specify locale, a string | string[] union isn't accepted.

I'm submitting a PR: #10842

mhegazy added a commit that referenced this issue Sep 13, 2016
@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Sep 13, 2016
@mhegazy mhegazy added this to the TypeScript 2.1 milestone Sep 13, 2016
kakaruna added a commit to kakaruna/TypeScript that referenced this issue Sep 26, 2016
* Factor out getRenameInfo

* Update Jakefile

* Correct emit comment for decorated class declaration

* Add tests and baselines

* Allow -Infinity as an enum property name

* Move allocators.ts to services.ts, meaning.ts to utilities.ts, and transpile functions to a new file transpile.ts

* Correct strings based on linting rules

* Force source-map-support to not have source maps

It fooled sorcery's incorrect check for sourceMappingURL into thinking
it had a source map.

Also up the error stack trace limit to 1000 to help future error
reporting.

* Only emit comment only once in module declaration with identifier path name

* Only emit comment once for export enum declaration

* When emitting react code, replace HTML numeric entities with their encoded characters

* Also decode entities when emitting attributes. Also, lexer should not process string escapes in jsx attributes.

* Use undefined, not null, to satisfy linter

* Update preferConstRule to use getCombined*X*Flags

now that they are exported.

* Remove another unused function in preferConstRule

* Yep, another unused function in preferConstRule

* Revert "Remove failing fourslash tests (may need to be restored and fixed)"

This reverts commit 2a60f79.

* Revert "Remove failing fourslash tests (may need to be restored and fixed)"

This reverts commit 2f9c9c9.

* Number is assignable to enum, even inside union

Previously, numbers were not assignable to enums that were inside a
union

* Test that number is assignable to enum in a union

* Use baselines for quick info tests to ease updates

* Fix duplicate baselines

* Use shorthand properties

* A shorthand ambient module should be considered as possibly exporting a value.

* Enum literal is assignable to enum, even inside union

Previously, only number literals were assignable to enums inside unions.

* Test that enum literal is assignable to an enum in a union

* Fix tests

* Accept baselines after merge

* Fix fourslash test

* Make preProcessFile public again

* Fix both new enum assignability predicates

And update error reporting baseline (new error is less elaborate)

* Fix classifier as well.

* Fix deferred export of array binding pattern

* Move code to a new module documentRegistry.ts

* Handle `OmittedExpression` nodes in binding patterns

* Surfacing method to get Completion Symbol instead of details for better extensibility

* Surfacing function in services.ts

* Assume outer variables are always initialized in control flow analysis

* Add regression tests

* Don't try to unlink folders

* Fix line endings

* Fix {Map,WeakMap}.prototype.set method signatures (microsoft#10694)

* Document `endOfChain`

* Adding method stub for the LanguageServiceShimProxy implementation

* method stub throwing an error for SessionClient

* lint: remove trailing whitespace in completions.ts

* Simplify parameters of `quickInfoIs`: `expectedtext` must be present and `expectedDocumentation` must be a string or ommitted, never null.

* minor changes from PR feedback

* Code cleanup and a few edge cases

* Disallow comma operator when LHS is pure

* Prevent duplicate entries from type references

* Added a STRATEGY placeholder for the --moduleResolution option

* Preserve type parameter types in narrowing

* Add regression test

* Fix ECMA-402 declarations (issue microsoft#10618)

1. Make String.prototype.localeCompare's `locales` parameter optional,
   so `undefined` is allowed.
2. Declare the `locales` parameter as a `string | string[]` union
   instead of using overloads. Having separate overloads for `string`
   and `string[]` unnecessarily prevents passing a `string | string[]`.
   (These overloads predate the introduction of union types.)

* Accept new baselines

* Handle const binding elements with initializers correctly

* Add tests

* Update tests

* Address PR comments

1. Cache results of isEnumTypeRelatedTo
2. Make numeric literal assignment stricter again.
3. Use isEnumRelatedTo for comparing enums to each other. This provides
the previous semi-structural semantics.
4. Because of the new distinction between computed enums (no union
members) and union enums (no computed values => a union of enum
literals), some semi-structural code moves out to the body of
`isRelatedTo`.

* More tests of enum assignability

1. Numeric literal <-> enum literal assignability
2. Computed enum <-> union enum assignability
3. Also rebaseline error reporting of existing enum cases.

* Fix lint

* Update baselines

* PR feedback

* Fix issue with helper emit.

Fixes microsoft#10800

* Quick bail out when narrowing type any by equality

* Add regression test

* Support export default for target=ES5/module=ES6.

Fixes microsoft#10855

* Update failing test baseline

* Enum assignability:loosen numbers+tighten computed

1. All numbers and numeric literals are assignable to all enums and enum
literals.
2. Computed enums are no longer assignable to anything except
themselves, even if they would otherwise be "semi-structurally"
compatible.

* Update baselines for updated enum assignability

* Fix missing asteriskToken for target=es6/module=amd.

Fixes microsoft#10857.

* Computed enum assignability is semi-structural

* Update baselines

* Disallow left comma operator operands which don't have side effects

* No widening of inferred type when initializer has a type assertion

* Accept new baselines

* Add test

* Fix missing final label.

Fixes microsoft#10876

* Implement NavigateTo for single files, instead of the project.

* Get rid of BOM

* Consume exceptions when checking for import completions

* Also wrap getEffectiveTypeRoots in import completion code

* Added test for scenario.

* Fix captured block scope variables in downlevel async.

Fixes microsoft#10889

* Remove unnecessary parentheses

* PR feedback and clean up.

* Add stackTraceLimit; update harness/tsconfig.json

1. Add stackTraceLimit argument to runtests.
2. Copy missing compiler files from compiler/tsconfig.json to
harness/tsconfig.json

* Simplify quick-info tests

* Flip check, add SEF cases

* Baseline update

* Add JSX to SEF exprs

* Fix build tasks for iocapture

* Added comments for __generator, reduced overall size of helper

* Add Code of Conduct

* Removed Code of Conduct from contributing, since readme is more obvious

* Fix build error caused by merge

* move to contribute header

* More PR feedback

* Use diffrent error message for namespaces unexported members

* Update other defintions of findIndex

* Rename function and use `directorySeparator` variables

* Remove duplicate test

* Lint

* Fix issue 10843

* add test for issue 10843

* Match number and string literal types to number and string in inference

* Add readonly typings for Set and Map

Similar to ReadonlyArray, these typings remove the mutation methods from Set and Map so that they can only be initialized by the constructor.

* Accept new baselines

* Add regression test

* Fix test issues due to an out of order merge

* Fix super in down-level async method

* add release-2.0

* More PR feedback

* Address CR feedback

* Allow type and NS references to UMD globals from modules

Fixes microsoft#10638

* Add comment explaining a magic constant in parser

* Fixing formatting

* revert tests

* revert fix for generated files

* update findIndex for es5 typed arrays

* Make declaration emit test name consistent

* Update baselines

* Removed constructor typings which can't be used

Also corrected some parameter names.

* indenting

* Add tests

* Serialize type alias when type alias symbol is not accessible

* Update baselines

* Address PR

* Fix build break: difference result from treating things as literal type

* Address PR: Update comment

* change error message for assigning from object

* Fix Reflect has method signature(s) per issue microsoft#10949 initial report

* fix linting error

* Output the help message line by line.

* Pass the jake test

* Handle msising tags for JsDoc nodes

* Add back getSourceFile and mark it as deprecated

* Use TypeFlags.Never to check for 'never' type

* Use silent never type in control flow loop analysis

* Add regression test

* Accept new baselines

* Take a few changes.

* fix typo

* remove extra files

* Correctly remove stale .error.txt baselines

* Fix downlevel async hoisting

* Fix some issues with module ES6/target ES5

* Fix missing visit of expression in for..of
@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
Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Fixed A PR has been merged for this issue Help Wanted You can do this
Projects
None yet
Development

No branches or pull requests

2 participants