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

As operator by RyanCavanaugh · Pull Request #3201 · microsoft/TypeScript · GitHub

As operator - #3201

Closed
Ryan Cavanaugh (RyanCavanaugh) wants to merge 12 commits into
microsoft:masterfrom
RyanCavanaugh:as_operator
Closed

As operator#3201
Ryan Cavanaugh (RyanCavanaugh) wants to merge 12 commits into
microsoft:masterfrom
RyanCavanaugh:as_operator

Conversation

Copy link
Copy Markdown
Member

Implements as operator as suggested in #296.

Comment thread src/compiler/types.ts Outdated

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

why do you need this token?

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.

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

Don't think we need to store the as token.

Copy link
Copy Markdown
Member

Can we have a few tests for templates in tests/cases/.../es6/templates/?

var x = `${123 + 456 as number}`;
var y = `leading ${123 + 456 as number}`;
var y = `${123 + 456 as number} trailing`;
var x = `Hello ${123} World` as string;
var y = `Hello` as string;
var z = 1 + `${1} end of string` as string;
declare function tag(...x: any[]): any;

var x = tag `Hello ${123} World` as string;
var y = tag `Hello` as string;

Copy link
Copy Markdown
Contributor

Should we add formating rule for as?
For example:

var x = 42   as string;

will become

var x = 42 as string;

Copy link
Copy Markdown
Contributor

Could we add some tests:

var a = 20;
var b = a as string;
var as = "hello";
var as1 = as as string;

Copy link
Copy Markdown
Contributor

I'm just wondering if we can support inferred type assertions and if it is a good idea?

Instead of:

(foo as Foo).(bar as Bar).text

We could just use:

foo.bar.text

And it will infer the type by looking at the last property.

Copy link
Copy Markdown
Member

Should we add formating rule for as?

If by formatting rule, you mean a restriction, yes (good catch!), but not related to the example you just gave (nowhere else in the language do we differentiate whitespace on the same line). Consider the following:

class Foo { }
declare function as(...args: any[]);

// Example 1
var x = 10
as `Hello world`

// Example 2
var y = 20
as(Foo);

Example 1 is not as much of a problem; you can't use a template string as a type.

Example 2 suffers from potentially the same problem as #2995. as would ordinarily be a function call with a constructor function, but here it is a type assertion on 20 to Foo.

Copy link
Copy Markdown
Member

Yui (@yuit) Ah, by formatting rule, you meant in the LS - still, glad we caught this.

Copy link
Copy Markdown
Contributor

Daniel Rosenwasser (@DanielRosenwasser) yes, I mean from LS side similar to:

function       foo() {}

become

function foo() {}

Though you example will be a good one to add as well.

Comment thread src/compiler/parser.ts Outdated

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.

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

else on the next line.

Copy link
Copy Markdown
Member Author

Any other feedback?

Comment thread src/compiler/parser.ts Outdated

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

just add a comment for this case.

Comment thread src/compiler/parser.ts Outdated

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.

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

else on the next line

Copy link
Copy Markdown
Member

You're currently not contextually typing the left-hand side of the as operator. So if you take

var x = (v => v) as (x: number) => number

v gets typed as any right now. This means that

var x = (v => v) as (x: number) => string

currently typechecks without a problem even though

var x = <(x: number) => string>(v => v)

gives an error.

Copy link
Copy Markdown
Member

Can we also have the following tests to demonstrate left-associativity of as?

asOperatorAssociativity01.ts

var x = 10 as number as any as string // should be okay

asOperatorAssociativity02.ts

var y = 10 as string as number; // should error

Copy link
Copy Markdown
Member Author

Anything else?

Copy link
Copy Markdown
Member

Are we leaving the services layer to a second pass? I can think of at least:

  • Formatting tests
  • Does as get occurrence highlighting to point out type assertion locations? (kind of useful; it's the point of static_cast etc. in C++).
  • Completion list tests for arrow functions:
class C<T> {
    constructor() {
        // C, T, and U should show up below.
        let f = <U>(x: any) => x as /**/ 
type A = any;
namespace n {
    type B = any;
    // A, B, T, and n should show up below.
    let f = <T>(x: any) => x as /**/ 
  • Completion list tests to ensure that you never have a new identifier location after an as.

Copy link
Copy Markdown
Member

I am working on a change such that keywords like interface must be followed by an identifier on the same line to be considered the start of an interface declaration.

But as is only a type assertion if it follows an identifier on the same line. So there's an ambiguity.

It seems like the appropriate thing to do is to treat it as an declaration named as.

You'll have to account for this with some tests like:

interface as { }
interface
as
{ }
interface
as({ })
namespace as {}
declare as {}
type as = number;

Comment thread src/compiler/parser.ts

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

Can you just check scanner.hasPrecedingLineBreak instead? I don't really like canParseSemicolon that much.

Copy link
Copy Markdown
Contributor

Ryan Cavanaugh (@RyanCavanaugh) is this ready to go in?

Copy link
Copy Markdown
Member Author

I'm merging this up with the JSX work, which should have a PR in a day or so

Copy link
Copy Markdown
Contributor

closing in favor of #3564

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.

9 participants


Back | FazBrowse Home | New Git URL