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

Allow object."field" syntax (see #7722) #9433

Closed
wants to merge 7 commits into from

Conversation

nadako
Copy link
Member

@nadako nadako commented May 16, 2020

Implements #7722.

typedef Schema = {
	final "$ref":String;
	var ?"enum":Int;
	var "default":{"$id":Int, "class":String};
}

var a:Schema = {
	"$ref": "hi",
	"enum": 42,
	"default": {"$id": 10, "class": "bye"}
};

Unresolved:

  • preserve quote status in EField? (also required for "field-starts-with-$" check, because o."$id" should be allowed)
  • disable reification for macro o."$id" (also needs EField quote status), macro {"$id": e} and macro : {"$id":Int} (needs quote status too) (anything else?)
  • special-character escaping for field and inlined locals (target-specific? some generic one could be provided), allow inlining object declarations with non-valid-identifier fields?
  • do we really accept full range of string characters? (newlines? unicode?)
  • allow such fields on classes? (technically might be needed for interop)

Tooling comments:

  • tmLanguage coloring breaks horribly
  • tokentree/formatter probably breaks?
  • vshaxe autocompletes unquoted fields (and thus invalid syntax)
  • vshaxe hover hints show unquoted fields

@nadako nadako requested a review from Simn May 16, 2020 20:46
@back2dos
Copy link
Member

do we want full range of string characters? (newlines? unicode?)

I'm glad you asked. I think we all agree that what we really want is this:

class Awesome {
  final "🔫🦈" = "LaserShark";
}

@nadako
Copy link
Member Author

nadako commented May 16, 2020

do we want full range of string characters? (newlines? unicode?)

I'm glad you asked. I think we all agree that what we really want is this:

class Awesome {
  final "🔫🦈" = "LaserShark";
}

I actually just tested this and have to regretfully inform that this doesn't work for class fields due to another sanity check pass :)

@nadako
Copy link
Member Author

nadako commented May 16, 2020

this doesn't work for class fields due to another sanity check pass

maybe it should tho, if we want to pass a class to some extern API that uses structural typing and weird field names...

@nadako
Copy link
Member Author

nadako commented May 16, 2020

Honestly, I feel like this raises more questions and surfaces more awkwardness (e.g. how to generate such fields on not-js) and is more breaking (if we change macro structures, as we should) than the @:native solution, but I'll let you guys decide.

@haxiomic
Copy link
Member

haxiomic commented May 16, 2020

Wow that was fast! Thanks @nadako. I'm excited for this one – often I load config files and pathnames into structures at compile time (many of which are not valid idents) so this change will massively improve things there!

@nadako
Copy link
Member Author

nadako commented May 16, 2020

Wow that was fast!

Yep, it took only 7 years ^^

@back2dos
Copy link
Member

Honestly, I feel like this raises more questions and surfaces more awkwardness (e.g. how to generate such fields on not-js)

Can we not have an ident validator in platform_config which defaults to Lexer.is_valid_identifier? And then the generator only gets idents it considers valid and then must see to it that it can generate them properly (as is already done to avoid clashing with target keywords).

and is more breaking (if we change macro structures, as we should) than the @:native solution, but I'll let you guys decide.

Hmmmm. The parser might as well swallow the distinction between quoted and unquoted fields (ok, for the time being we should keep it for object literal fields for compatibility, but the quote status should be ignored in the typer). Field names would be plain strings and the typer would check them via what's in platform_config. Bonus points for exposing that function as Context.isValidIdent or something.

@nadako
Copy link
Member Author

nadako commented May 17, 2020

The parser might as well swallow the distinction between quoted and unquoted fields

That's what happening at the moment, but it is a problem, because the information is lost at the point where it comes into reification functions (e.g. a."$field" will reify into EField(a, field) and not EField(a, "$field")). Another issue is this check, that should stay for non-quoted fields, but should be allowed for quoted ones.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants