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.

Sign up for GitHub

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

Jump to bottom

Fix shadowing Js.Json.t with type kind #6317

Merged
merged 7 commits into from
Jul 4, 2023
Merged

Fix shadowing Js.Json.t with type kind #6317

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0d46574
fix shadowing Js.Json.t with type kind
mununki Jul 1, 2023
68e6506
changelog
mununki Jul 1, 2023
7a8880b
type s instead kind
mununki Jul 3, 2023
a02f005
changelog
mununki Jul 3, 2023
bba4ec0
Revert "type s instead kind"
mununki Jul 4, 2023
695bb47
Kind.t
mununki Jul 4, 2023
7f3959d
changelog
mununki Jul 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
#### :rocket: New Feature
- Variants: Allow coercing from variant to variant, where applicable. https://github.com/rescript-lang/rescript-compiler/pull/6314

#### :boom: Breaking Change
- Fixed the issue of name collision between the newly defined Js.Json.t and the variant constructor in the existing Js.Json.kind type. To address this, the usage of the existing Js.Json.kind type can be updated to Js.Json.Kind.s. https://github.com/rescript-lang/rescript-compiler/pull/6317
Copy link
Collaborator

Choose a reason for hiding this comment

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

s / s / t

Copy link
Member Author

Choose a reason for hiding this comment

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

s / kind / t 7f3959d

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain why it's not t?

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought it's idiomatic for types

Copy link
Collaborator

Choose a reason for hiding this comment

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

It ended up being t, s was just a confusion.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is t.

type t

module Kind = {
  type json = t
  type t
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I see 😁


# 11.0.0-beta.3

#### :rocket: New Feature
Expand Down
31 changes: 17 additions & 14 deletions jscomp/others/js_json.res
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,16 @@ type rec t =
| Object(Js.Dict.t<t>)
| Array(array<t>)

type rec kind<_> =
| String: kind<Js_string.t>
| Number: kind<float>
| Object: kind<Js_dict.t<t>>
| Array: kind<array<t>>
| Boolean: kind<bool>
| Null: kind<Js_types.null_val>
module Kind = {
type json = t
type rec t<_> =
| String: t<Js_string.t>
| Number: t<float>
| Object: t<Js_dict.t<json>>
| Array: t<array<json>>
| Boolean: t<bool>
| Null: t<Js_types.null_val>
}

type tagged_t =
| JSONFalse
Expand Down Expand Up @@ -72,14 +75,14 @@ let classify = (x: t): tagged_t => {
}
}

let test = (type a, x: 'a, v: kind<a>): bool =>
let test = (type a, x: 'a, v: Kind.t<a>): bool =>
switch v {
| Number => Js.typeof(x) == "number"
| Boolean => Js.typeof(x) == "boolean"
| String => Js.typeof(x) == "string"
| Null => Obj.magic(x) === Js.null
| Array => Js_array2.isArray(x)
| Object => Obj.magic(x) !== Js.null && (Js.typeof(x) == "object" && !Js_array2.isArray(x))
| Kind.Number => Js.typeof(x) == "number"
| Kind.Boolean => Js.typeof(x) == "boolean"
| Kind.String => Js.typeof(x) == "string"
| Kind.Null => Obj.magic(x) === Js.null
| Kind.Array => Js_array2.isArray(x)
| Kind.Object => Obj.magic(x) !== Js.null && (Js.typeof(x) == "object" && !Js_array2.isArray(x))
}

let decodeString = json =>
Expand Down
21 changes: 12 additions & 9 deletions jscomp/others/js_json.resi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,17 @@ type rec t =
| Object(Js.Dict.t<t>)
| Array(array<t>)

/** Underlying type of a JSON value */
type rec kind<_> =
| String: kind<Js_string.t>
| Number: kind<float>
| Object: kind<Js_dict.t<t>>
| Array: kind<array<t>>
| Boolean: kind<bool>
| Null: kind<Js_types.null_val>
module Kind: {
type json = t
/** Underlying type of a JSON value */
type rec t<_> =
| String: t<Js_string.t>
| Number: t<float>
| Object: t<Js_dict.t<json>>
| Array: t<array<json>>
| Boolean: t<bool>
| Null: t<Js_types.null_val>
}

type tagged_t =
| JSONFalse
Expand All @@ -63,7 +66,7 @@ type tagged_t =
let classify: t => tagged_t

/** `test(v, kind)` returns `true` if `v` is of `kind`. */
let test: ('a, kind<'b>) => bool
let test: ('a, Kind.t<'b>) => bool

/** `decodeString(json)` returns `Some(s)` if `json` is a `string`, `None` otherwise. */
let decodeString: t => option<Js_string.t>
Expand Down
44 changes: 22 additions & 22 deletions jscomp/test/js_json_test.res
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,40 +145,40 @@ let () = {

/* Check that the given json value is an array and that its element
* a position [i] is equal to both the [kind] and [expected] value */
let eq_at_i = (type a, loc: string, json: J.t, i: int, kind: J.kind<a>, expected: a): unit => {
let eq_at_i = (type a, loc: string, json: J.t, i: int, kind: J.Kind.t<a>, expected: a): unit => {
let ty = J.classify(json)
switch ty {
| J.JSONArray(x) =>
let ty = J.classify(x[i])
switch kind {
| J.Boolean =>
| J.Kind.Boolean =>
switch ty {
| JSONTrue => eq(loc, true, expected)

| JSONFalse => eq(loc, false, expected)
| _ => false_(loc)
}
| J.Number =>
| J.Kind.Number =>
switch ty {
| JSONNumber(f) => eq(loc, f, expected)
| _ => false_(loc)
}
| J.Object =>
| J.Kind.Object =>
switch ty {
| JSONObject(f) => eq(loc, f, expected)
| _ => false_(loc)
}
| J.Array =>
| J.Kind.Array =>
switch ty {
| JSONArray(f) => eq(loc, f, expected)
| _ => false_(loc)
}
| J.Null =>
| J.Kind.Null =>
switch ty {
| JSONNull => true_(loc)
| _ => false_(loc)
}
| J.String =>
| J.Kind.String =>
switch ty {
| JSONString(f) => eq(loc, f, expected)
| _ => false_(loc)
Expand All @@ -196,18 +196,18 @@ let () = {
|> J.stringify
|> J.parseExn

eq_at_i(__LOC__, json, 0, J.String, "string 0")
eq_at_i(__LOC__, json, 1, J.String, "string 1")
eq_at_i(__LOC__, json, 2, J.String, "string 2")
eq_at_i(__LOC__, json, 0, J.Kind.String, "string 0")
eq_at_i(__LOC__, json, 1, J.Kind.String, "string 1")
eq_at_i(__LOC__, json, 2, J.Kind.String, "string 2")
()
}

let () = {
let json = ["string 0", "string 1", "string 2"] |> J.stringArray |> J.stringify |> J.parseExn

eq_at_i(__LOC__, json, 0, J.String, "string 0")
eq_at_i(__LOC__, json, 1, J.String, "string 1")
eq_at_i(__LOC__, json, 2, J.String, "string 2")
eq_at_i(__LOC__, json, 0, J.Kind.String, "string 0")
eq_at_i(__LOC__, json, 1, J.Kind.String, "string 1")
eq_at_i(__LOC__, json, 2, J.Kind.String, "string 2")
()
}

Expand All @@ -216,9 +216,9 @@ let () = {
let json = a |> J.numberArray |> J.stringify |> J.parseExn

/* Loop is unrolled to keep relevant location information */
eq_at_i(__LOC__, json, 0, J.Number, a[0])
eq_at_i(__LOC__, json, 1, J.Number, a[1])
eq_at_i(__LOC__, json, 2, J.Number, a[2])
eq_at_i(__LOC__, json, 0, J.Kind.Number, a[0])
eq_at_i(__LOC__, json, 1, J.Kind.Number, a[1])
eq_at_i(__LOC__, json, 2, J.Kind.Number, a[2])
()
}

Expand All @@ -227,9 +227,9 @@ let () = {
let json = a |> Array.map(float_of_int) |> J.numberArray |> J.stringify |> J.parseExn

/* Loop is unrolled to keep relevant location information */
eq_at_i(__LOC__, json, 0, J.Number, float_of_int(a[0]))
eq_at_i(__LOC__, json, 1, J.Number, float_of_int(a[1]))
eq_at_i(__LOC__, json, 2, J.Number, float_of_int(a[2]))
eq_at_i(__LOC__, json, 0, J.Kind.Number, float_of_int(a[0]))
eq_at_i(__LOC__, json, 1, J.Kind.Number, float_of_int(a[1]))
eq_at_i(__LOC__, json, 2, J.Kind.Number, float_of_int(a[2]))
()
}

Expand All @@ -238,9 +238,9 @@ let () = {
let json = a |> J.booleanArray |> J.stringify |> J.parseExn

/* Loop is unrolled to keep relevant location information */
eq_at_i(__LOC__, json, 0, J.Boolean, a[0])
eq_at_i(__LOC__, json, 1, J.Boolean, a[1])
eq_at_i(__LOC__, json, 2, J.Boolean, a[2])
eq_at_i(__LOC__, json, 0, J.Kind.Boolean, a[0])
eq_at_i(__LOC__, json, 1, J.Kind.Boolean, a[1])
eq_at_i(__LOC__, json, 2, J.Kind.Boolean, a[2])
()
}

Expand Down
3 changes: 3 additions & 0 deletions lib/es6/js_json.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import * as Caml_option from "./caml_option.js";

var Kind = {};

function classify(x) {
var ty = typeof x;
if (ty === "string") {
Expand Down Expand Up @@ -158,6 +160,7 @@ function deserializeUnsafe(s) {
}

export {
Kind ,
classify ,
test ,
decodeString ,
Expand Down
3 changes: 3 additions & 0 deletions lib/js/js_json.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

var Caml_option = require("./caml_option.js");

var Kind = {};

function classify(x) {
var ty = typeof x;
if (ty === "string") {
Expand Down Expand Up @@ -157,6 +159,7 @@ function deserializeUnsafe(s) {
return patch(JSON.parse(s));
}

exports.Kind = Kind;
exports.classify = classify;
exports.test = test;
exports.decodeString = decodeString;
Expand Down
Loading