winglang / wing

A programming language for the cloud ☁️ A unified programming model, combining infrastructure and runtime code into one language ⚡
https://winglang.io
Other
4.76k stars 187 forks source link

fix(compiler): optional macro chaining #6659

Open tsuf239 opened 3 weeks ago

tsuf239 commented 3 weeks ago

closes #4470

I inserted a default value for optional maps, json, sets, and structs below you can find the compilation output of the following wing code, comments and newlines added later and not a part of the original compilation output.

struct Result {
  item: Json?;
  items: Array<Json>?;
  mapItem: Map<str>?;
  mapItems: Array<Map<str>>?;
  setItem: Set<num>?;
  setItems: Array<Set<num>>?;
  structItem: Result?;
  structItems: Array<Result>?;
}

test "optional chaining macros" {
  let result = Result {};

  expect.equal(result.item?.tryGet("id"), nil);
  expect.equal(result.item?.has("id"), false);

  expect.equal(result.items?.tryAt(0)?.tryGet("id"), nil);
  expect.equal(result.items?.tryAt(0)?.has("id"), false);

  expect.equal(result.mapItem?.tryGet("a"), nil);
  expect.equal(result.mapItem?.has("id"), false);

  expect.equal(result.mapItems?.tryAt(0)?.tryGet("id"), nil);
  expect.equal(result.mapItems?.tryAt(0)?.has("id"), false);

  expect.equal(result.setItem?.size, nil);
  expect.equal(result.setItem?.has(6), nil);

  expect.equal(result.setItems?.tryAt(0)?.size, nil);
  expect.equal(result.setItems?.tryAt(0)?.has(6), nil);

  expect.equal(result.structItem?.item, nil);
  expect.equal(result.structItems?.tryAt(0)?.item, nil);
}
       // expect.equal(result.item?.tryGet("id"), nil);
      (await $expect_Util.equal(((result.item ?? {}))?.["id"], undefined));
      // expect.equal(result.item?.has("id"), false);
      (await $expect_Util.equal(((obj, key) => { return obj.hasOwnProperty(key); })((result.item ?? {}),"id"), false));

      // expect.equal(result.items?.tryAt(0)?.tryGet("id"), nil);
      (await $expect_Util.equal((((result.items ?? []).at(0) ?? {}))?.["id"], undefined));
      // expect.equal(result.items?.tryAt(0)?.has("id"), false);
      (await $expect_Util.equal(((obj, key) => { return obj.hasOwnProperty(key); })(((result.items ?? []).at(0) ?? {}),"id"), false));

      // expect.equal(result.mapItem?.tryGet("a"), nil);
      (await $expect_Util.equal(((result.mapItem ?? new Map()))["a"], undefined));
      // expect.equal(result.mapItem?.has("id"), false);
      (await $expect_Util.equal(("id" in ((result.mapItem ?? new Map()))), false));

      // expect.equal(result.mapItems?.tryAt(0)?.tryGet("id"), nil);
      (await $expect_Util.equal((((result.mapItems ?? []).at(0) ?? new Map()))["id"], undefined));
       // expect.equal(result.mapItems?.tryAt(0)?.has("id"), false);
      (await $expect_Util.equal(("id" in (((result.mapItems ?? []).at(0) ?? new Map()))), false));

       // expect.equal(result.setItem?.size, nil);
      (await $expect_Util.equal(result.setItem?.size, undefined));
      // expect.equal(result.setItem?.has(6), nil);
      (await $expect_Util.equal((await result.setItem?.has?.(6)), undefined));

      // expect.equal(result.setItems?.tryAt(0)?.size, nil);
      (await $expect_Util.equal((result.setItems ?? []).at(0)?.size, undefined));
      //  expect.equal(result.setItems?.tryAt(0)?.has(6), nil);
      (await $expect_Util.equal((await (result.setItems ?? []).at(0)?.has?.(6)), undefined));

      //  expect.equal(result.structItem?.item, nil);
      (await $expect_Util.equal(result.structItem?.item, undefined));
     // expect.equal(result.structItems?.tryAt(0)?.item, nil);
      (await $expect_Util.equal((result.structItems ?? []).at(0)?.item, undefined));

the other thing that needs to be changed in the issue's example is to change the get to tryGet since the key we try to reach is non-existent.

Checklist

By submitting this pull request, I confirm that my contribution is made under the terms of the Wing Cloud Contribution License.

github-actions[bot] commented 3 weeks ago

Thanks for opening this pull request! :tada: Please consult the contributing guidelines for details on how to contribute to this project. If you need any assistance, don't hesitate to ping the relevant owner over Discord.

Topic Owner
Wing SDK and utility APIs @chriscbr
Wing Console @ainvoner, @skyrpex, @polamoros
JSON, structs, primitives and collections @hasanaburayyan
Platforms and plugins @hasanaburayyan
Frontend resources (website, react, etc) @tsuf239
Language design @chriscbr
VSCode extension and language server @markmcculloh
Compiler architecture, inflights, lifting @yoav-steinberg
Wing Testing Framework @tsuf239
Wing CLI @markmcculloh
Build system, dev environment, releases @markmcculloh
Library Ecosystem @chriscbr
Documentation @hasanaburayyan
SDK test suite @tsuf239
Examples @hasanaburayyan
Wing Playground @eladcon
monadabot commented 3 weeks ago

Console preview environment is available at https://wing-console-pr-6659.fly.dev :rocket:

Last Updated (UTC) 2024-07-01 16:21
tsuf239 commented 3 weeks ago

here is the new compilation output:

 // expect.equal(result.item?.tryGet("id"), nil);
await $expect_Util.equal(result.item === undefined ? undefined : result.item?.["id"], undefined);
 // expect.equal(result.item?.has("id"), nil);
      (await $expect_Util.equal((result.item === undefined ? undefined : ((obj, key) => { return obj.hasOwnProperty(key); })(result.item,"id")), undefined));

 // expect.equal(result.items?.tryAt(0)?.tryGet("id"), nil);
      (await $expect_Util.equal((result.items?.at(0) === undefined ? undefined : (result.items?.at(0))?.["id"]), undefined));
 // expect.equal(result.items?.tryAt(0)?.has("id"), nil);
      (await $expect_Util.equal((result.items?.at(0) === undefined ? undefined : ((obj, key) => { return obj.hasOwnProperty(key); })(result.items?.at(0),"id")), undefined));

  // expect.equal(result.mapItem?.tryGet("a"), nil);
      (await $expect_Util.equal((result.mapItem === undefined ? undefined : (result.mapItem)["a"]), undefined));
  // expect.equal(result.mapItem?.has("id"), nil);
      (await $expect_Util.equal((result.mapItem === undefined ? undefined : ("id" in (result.mapItem))), undefined));

  // expect.equal(result.mapItems?.tryAt(0)?.tryGet("id"), nil);
      (await $expect_Util.equal((result.mapItems?.at(0) === undefined ? undefined : (result.mapItems?.at(0))["id"]), undefined));
  // expect.equal(result.mapItems?.tryAt(0)?.has("id"), nil);
      (await $expect_Util.equal((result.mapItems?.at(0) === undefined ? undefined : ("id" in (result.mapItems?.at(0)))), undefined));

  // expect.equal(result.setItem?.size, nil);
      (await $expect_Util.equal(result.setItem?.size, undefined));
  // expect.equal(result.setItem?.has(6), nil);
      (await $expect_Util.equal((await result.setItem?.has?.(6)), undefined));

  // expect.equal(result.setItems?.tryAt(0)?.size, nil);
      (await $expect_Util.equal((result.setItems === undefined ? undefined : result.setItems?.at(0))?.size, undefined));
  // expect.equal(result.setItems?.tryAt(0)?.has(6), nil);
      (await $expect_Util.equal((await (result.setItems === undefined ? undefined : result.setItems?.at(0))?.has?.(6)), undefined));

  // expect.equal(result.structItem?.item, nil);
      (await $expect_Util.equal(result.structItem?.item, undefined));
  // expect.equal(result.structItems?.tryAt(0)?.item, nil);
      (await $expect_Util.equal((result.structItems === undefined ? undefined : result.structItems?.at(0))?.item, undefined));
github-actions[bot] commented 14 hours ago

Hi,

This PR has not seen activity in 20 days. Therefore, we are marking the PR as stale for now. It will be closed after 7 days. If you need help with the PR, do not hesitate to reach out in the winglang community Discord. Feel free to re-open this PR when it is still relevant and ready to be worked on again. Thanks!

tsuf239 commented 4 hours ago

reached to the cleanest solution by changing map's has into the js version (abstract) and json's has to accept optional (there is nothing we can do in the justification process to get more precise result, more than changing the macros their self to be more precise)

tsuf239 commented 4 hours ago
 // expect.equal(result.item?.tryGet("id"), nil);
 (await $expect_Util.equal((result.item)?.["id"], undefined));
//  expect.equal(result.item?.has("id"), false);
(await $expect_Util.equal(((obj, key) => { return obj?.hasOwnProperty(key); })(result.item,"id"), undefined));

  // expect.equal(result.items?.tryAt(0)?.tryGet("id"), nil);
      (await $expect_Util.equal((result.items?.at(0))?.["id"], undefined));
  // expect.equal(result.items?.tryAt(0)?.has("id"), false);
 (await $expect_Util.equal(((obj, key) => { return obj?.hasOwnProperty(key); })(result.items?.at(0),"id"), undefined));

  // expect.equal(result.mapItem?.tryGet("a"), nil);
 (await $expect_Util.equal((result.mapItem)?.["a"], undefined));
  // expect.equal(result.mapItem?.has("id"), false);
      (await $expect_Util.equal((await result.mapItem?.has?.("id")), undefined));

  // expect.equal(result.mapItems?.tryAt(0)?.tryGet("id"), nil);
      (await $expect_Util.equal((result.mapItems?.at(0))?.["id"], undefined));
  // expect.equal(result.mapItems?.tryAt(0)?.has("id"), false);
      (await $expect_Util.equal((await result.mapItems?.at(0)?.has?.("id")), undefined));

  // expect.equal(result.setItem?.size, nil);
      (await $expect_Util.equal(result.setItem?.size, undefined));
  // expect.equal(result.setItem?.has(6), nil);
      (await $expect_Util.equal((await result.setItem?.has?.(6)), undefined));

  // expect.equal(result.setItems?.tryAt(0)?.size, nil);
      (await $expect_Util.equal(result.setItems?.at(0)?.size, undefined));
  // expect.equal(result.setItems?.tryAt(0)?.has(6), nil);
      (await $expect_Util.equal((await result.setItems?.at(0)?.has?.(6)), undefined));

 // expect.equal(result.structItem?.item, nil);
      (await $expect_Util.equal(result.structItem?.item, undefined));
 // expect.equal(result.structItems?.tryAt(0)?.item, nil);
      (await $expect_Util.equal(result.structItems?.at(0)?.item, undefined));     
Chriscbr commented 3 hours ago

reached to the cleanest solution by changing map's has into the js version (abstract) and json's has to accept optional (there is nothing we can do in the justification process to get more precise result, more than changing the macros their self to be more precise)

I still think we should look into handle this automatically through jsification. Here's an example of another macro that's still not handled, so it fails to compile:

let makeArray = (): Array<num> => {
  log("makeArray called");
  return [1, 2, 3];
};
let makeArray2 = (): Array<num>? => {
  log("makeArray2 called");
  return nil;
};

log(makeArray().at(1));
log(makeArray2()?.at(1) ?? -1);

We have over 80 macros in the standard library (and growing), and writing tests for each one in an optional chaining position feels like effort worth avoiding.


I'm wondering if you can give another try with the code generation approach from this comment? https://github.com/winglang/wing/pull/6659#discussion_r1632581331

Btw - I wonder if we can make the emitted code a bit more readable by, say, generating a global function for each macro (I believe a lot of JavaScript bundlers do this):

"use strict";
// since Array.at is used in the Wing source code and the method is a macro,
// the compiler emits a global function named `__Array_at`
// if `Array.at` is called multiple times, `__Array_at` is only added once
let __Array_at = function (skipIfNil, $self$, ...$args$) {
  // optional chaining behavior
  if (skipIfNil && $self$ == undefined) return $self$;
  // return <macro text>;
  return ((arr, index) => { if (index < 0 || index >= arr.length) throw new Error("Index out of bounds"); return arr[index]; })($self$, $args$);
}
const makeArray = (() => {
  console.log("makeArray called");
  return [1, 2, 3];
});
const makeArray2 = (() => {
  console.log("makeArray2 called");
  return undefined;
});
console.log(__Array_at(false, makeArray(), 1));
console.log(__Array_at(true, makeArray2(), 1) ?? (-1));