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

Json: schema validation and assignment to struct #3139

Closed
Tracked by #2999
staycoolcall911 opened this issue Jun 29, 2023 · 4 comments · Fixed by #3648
Closed
Tracked by #2999

Json: schema validation and assignment to struct #3139

staycoolcall911 opened this issue Jun 29, 2023 · 4 comments · Fixed by #3648
Assignees
Labels
🛠️ compiler Compiler 📜 lang-spec-impl Appears in the language spec roadmap

Comments

@staycoolcall911
Copy link
Contributor

staycoolcall911 commented Jun 29, 2023

The following sections should be added to the language reference:

1.1.4.5 Schema validation

All fromJson() methods will validate that the runtime type is compatible with the target type in
order to ensure type safety (at a runtime cost):

```TS
str.fromJson(jsonNumber);      // RUNTIME ERROR: unable to parse number `123` as a string.
num.fromJson(Json "\"hello\""); // RUNTIME ERROR: unable to parse string "hello" as a number

For each fromJson(), there is a tryFromJson() method which returns an optional T? which
indicates if parsing was successful or not:

let s = str.tryFromJson(myJson) ?? "invalid string";
1.1.4.7 Assignment to user-defined structs

All structs also have a fromJson() method that can be used to parse Json into a
struct:

struct Contact {
  first: str;
  last: str;
  phone: str?;
}

let j = Json { first: "Wing", last: "Lyly" };
let myContact = Contact.fromJson(j);
assert(myContact.first == "Wing");

When a Json is parsed into a struct, the schema will be validated to ensure the result is
type-safe:

let p = Json { first: "Wing", phone: 1234 };
Contact.fromJson(p);
// RUNTIME ERROR: unable to parse Contact:
// - field "last" is required and missing
// - field "phone" is expected to be a string, got number.
@staycoolcall911 staycoolcall911 added 📜 lang-spec-impl Appears in the language spec roadmap 🛠️ compiler Compiler labels Jun 29, 2023
mergify bot pushed a commit that referenced this issue Jun 29, 2023
This PR introduces an up-to-date language reference - all wishlist/roadmap feature mentions were deleted and instead put into new/existing issues, with a link in a newly added "Roadmap" paragraph at the end of relevant sections..
All code samples (Wing and TypeScript) were tested and should successfully compile.

[Rendered version of the language reference](https://github.com/winglang/wing/blob/urib/lang-ref/docs/docs/03-language-guide/90-reference.md#1149-roadmap).

Fixes #2420 

### TODO
- [ ] I'll remove all typescript code samples in a follow-up PR

### Misc
- [x] Changed the rust debugger to debug the current open `.w` file by default (relevant contributor guide doc updated).
- [x] Small fix of a couple of tree-sitter test headlines.
- [x] Fixes #2696 - by defining the behavior of `for` loops.

### I simply removed the following spec features/requirements:
1. In 1.1.4.8 Json logging
“It is also legal to just log a json object”
2. In 1.2 Utility Functions:
“The above functions can accept variadic arguments of any type except `throw` which only accepts one argument and that is the message to be contained in the error.”
3. In 1.4 Storage modifiers:
“The name of any static data member and static member function must be different from the name of the containing class regardless of the casing.”
4. In 2.6 For:
“Type annotation after an iteratee (left hand side of **in**) is optional.”
5. In 3.2 Classes:
“Optionals are initialized to `nil` if omitted, unless the type is `nil?`, which in that case, absent initialization is a compile error.<br/>
Member function and field access in constructor with the "this" keyword before
all fields are initialized is invalid and would throw a compile error.<br/>
In other words, the `this` keyword is immutable to its field access operator `.`
before all the member fields are properly initialized. The behavior is similar to JavaScript and TypeScript in their "strict" mode.”
6. In 3.3 Preflight classes:
Scope and ID are “both overrideable by user-defined ones in constructor.”
7. In 3.8 Enumeration:
“Last comma is optional in single line definitions but required in multi line definitions.”
“`nameof` operator”
8. Entire section: 6.1.2 Shell strings
9. Entire section: 6.4 Kitchen Sink
10. Entire section: Inspiration (last section)

### Issues opened/changed to track spec completeness:
- [x] Opened issues:
	- [x] #3121 
	- [x] #3123 
	- [x] #3129
	- [x] #3139 
	- [x] #3140 
	- [x] #3142 
- [x] Changed description:
	- [x] #108 
	- [x] #2103 
	- [x] #116 
	- [x] #125 
	- [x] #128
	- [x] #129 
	- [x] #130
	- [x] #1737 

## Checklist

- [x] Title matches [Winglang's style guide](https://docs.winglang.io/contributors/pull_requests#how-are-pull-request-titles-formatted)
- [x] Description explains motivation and solution
- [ ] Tests added (always)
- [x] Docs updated (only required for features)
- [ ] Added `pr/e2e-full` label if this feature requires end-to-end testing

*By submitting this pull request, I confirm that my contribution is made under the terms of the [Monada Contribution License](https://docs.winglang.io/terms-and-policies/contribution-license.html)*.
revitalbarletz pushed a commit that referenced this issue Jul 3, 2023
This PR introduces an up-to-date language reference - all wishlist/roadmap feature mentions were deleted and instead put into new/existing issues, with a link in a newly added "Roadmap" paragraph at the end of relevant sections..
All code samples (Wing and TypeScript) were tested and should successfully compile.

[Rendered version of the language reference](https://github.com/winglang/wing/blob/urib/lang-ref/docs/docs/03-language-guide/90-reference.md#1149-roadmap).

Fixes #2420 

### TODO
- [ ] I'll remove all typescript code samples in a follow-up PR

### Misc
- [x] Changed the rust debugger to debug the current open `.w` file by default (relevant contributor guide doc updated).
- [x] Small fix of a couple of tree-sitter test headlines.
- [x] Fixes #2696 - by defining the behavior of `for` loops.

### I simply removed the following spec features/requirements:
1. In 1.1.4.8 Json logging
“It is also legal to just log a json object”
2. In 1.2 Utility Functions:
“The above functions can accept variadic arguments of any type except `throw` which only accepts one argument and that is the message to be contained in the error.”
3. In 1.4 Storage modifiers:
“The name of any static data member and static member function must be different from the name of the containing class regardless of the casing.”
4. In 2.6 For:
“Type annotation after an iteratee (left hand side of **in**) is optional.”
5. In 3.2 Classes:
“Optionals are initialized to `nil` if omitted, unless the type is `nil?`, which in that case, absent initialization is a compile error.<br/>
Member function and field access in constructor with the "this" keyword before
all fields are initialized is invalid and would throw a compile error.<br/>
In other words, the `this` keyword is immutable to its field access operator `.`
before all the member fields are properly initialized. The behavior is similar to JavaScript and TypeScript in their "strict" mode.”
6. In 3.3 Preflight classes:
Scope and ID are “both overrideable by user-defined ones in constructor.”
7. In 3.8 Enumeration:
“Last comma is optional in single line definitions but required in multi line definitions.”
“`nameof` operator”
8. Entire section: 6.1.2 Shell strings
9. Entire section: 6.4 Kitchen Sink
10. Entire section: Inspiration (last section)

### Issues opened/changed to track spec completeness:
- [x] Opened issues:
	- [x] #3121 
	- [x] #3123 
	- [x] #3129
	- [x] #3139 
	- [x] #3140 
	- [x] #3142 
- [x] Changed description:
	- [x] #108 
	- [x] #2103 
	- [x] #116 
	- [x] #125 
	- [x] #128
	- [x] #129 
	- [x] #130
	- [x] #1737 

## Checklist

- [x] Title matches [Winglang's style guide](https://docs.winglang.io/contributors/pull_requests#how-are-pull-request-titles-formatted)
- [x] Description explains motivation and solution
- [ ] Tests added (always)
- [x] Docs updated (only required for features)
- [ ] Added `pr/e2e-full` label if this feature requires end-to-end testing

*By submitting this pull request, I confirm that my contribution is made under the terms of the [Monada Contribution License](https://docs.winglang.io/terms-and-policies/contribution-license.html)*.
@hasanaburayyan
Copy link
Collaborator

@staycoolcall911 this needs bumped up to P1 before Beta right? Well at least the struct.fromJson() bit

@staycoolcall911
Copy link
Contributor Author

Yes, definitely, this came up much during dogfooding. Bumping to p1

@hasanaburayyan hasanaburayyan self-assigned this Jul 15, 2023
@hasanaburayyan
Copy link
Collaborator

hasanaburayyan commented Aug 11, 2023

@mergify mergify bot closed this as completed in #3648 Aug 12, 2023
mergify bot pushed a commit that referenced this issue Aug 12, 2023
The changes in this PR make it possible to call `fromJson` on any compatible struct definition. 

## Implementation notes:
Previously we treated the jsification of structs as a no-op since there is not a JS equivalent. So with this change structs now JSify into a `Struct` file that contains a class with a static `jsonSchema` and a `fromJson` function which will allow for field validation at runtime. 

The schema generated adheres to: https://json-schema.org/understanding-json-schema/

take this simple example Wing code:
```js
struct MyStruct {
	myField: str;
	myOtherField: num;
}
```

this will now generate a JS file named `MyStruct.Struct.js` which looks like this:
```js
module.exports = function(stdStruct, fromInline) {
  class MyStruct {
    static jsonSchema() {
      return {
        id: "/MyStruct",
        type: "object",
        properties: {
          myField: { type: "string" },
          myOtherField: { type: "number" },
        },
        required: [
          "myField",
          "myOtherField",
        ],
        $defs: {
        }
      }
    }
    static fromJson(obj) {
      return stdStruct._validate(obj, this.jsonSchema())
    }
    static _toInflightType(context) {
      return fromInline(`require("./MyStruct.Struct.js")(${ context._lift(stdStruct) })`);
    }
  }
  return MyStruct;
};

```

The piece that brings this all together is the addition of the `Struct` class in our std that only has a `fromJson()` methods at the moment that is a Wing macro. The macro just calls the `fromJson()` method in the generated javascript.

### Misc
We want to stop the user at compile time from calling `fromJson` on a struct that cannot be represented by a Json value ie
```js
struct MyStruct {
	b: cloud.Bucket;
}
let j = {};
MyStruct.fromJson(j);
```

to prevent this I added a check in the typechecker for structs to confirm that if `fromJson` is called that all the fields in the struct are valid for conversion attempt

See image below for error when attempting:
<img width="664" alt="image" src="https://github.com/winglang/wing/assets/45375125/785a2fa6-8823-4fa2-aaa5-4bc8f7ed597f">


Closes: #3653
Closes: #3139

## TODO:
- [x] separate the work done here and the remaining work into different tickets.
- [x] update language reference

## Followup issues that are out of scope for this PR:
- #3792
- #3790
- #3789
- #3788

## Checklist

- [x] Title matches [Winglang's style guide](https://www.winglang.io/contributing/start-here/pull_requests#how-are-pull-request-titles-formatted)
- [x] Description explains motivation and solution
- [x] Tests added (always)
- [x] Docs updated (only required for features)
- [x] Added `pr/e2e-full` label if this feature requires end-to-end testing

*By submitting this pull request, I confirm that my contribution is made under the terms of the [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*.
@monadabot
Copy link
Contributor

Congrats! 🚀 This was released in Wing 0.25.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🛠️ compiler Compiler 📜 lang-spec-impl Appears in the language spec roadmap
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants