From 0b73a602306ef0496603a215547dd2d9bb840cde Mon Sep 17 00:00:00 2001
From: Tsuf Cohen <39455181+tsuf239@users.noreply.github.com>
Date: Fri, 14 Jun 2024 20:25:39 +0300
Subject: [PATCH] fix(compiler): `Json?` can be provided where `Json` is
 expected (#6679)

fixes: #6061

## Checklist

- [ ] Title matches [Winglang's style guide](https://www.winglang.io/contributing/start-here/pull_requests#how-are-pull-request-titles-formatted)
- [ ] Description explains motivation and solution
- [ ] Tests added (always)
- [ ] 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 [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*.
---
 examples/tests/invalid/json_is_not_nil.test.w | 27 +++++++++++++++++++
 examples/tests/sdk_tests/resource/call.test.w |  4 +--
 examples/tests/sdk_tests/table/get.test.w     |  4 +--
 libs/wingc/src/type_check.rs                  |  7 +++--
 tools/hangar/__snapshots__/invalid.ts.snap    | 15 +++++++++++
 5 files changed, 51 insertions(+), 6 deletions(-)
 create mode 100644 examples/tests/invalid/json_is_not_nil.test.w

diff --git a/examples/tests/invalid/json_is_not_nil.test.w b/examples/tests/invalid/json_is_not_nil.test.w
new file mode 100644
index 00000000000..8576224742b
--- /dev/null
+++ b/examples/tests/invalid/json_is_not_nil.test.w
@@ -0,0 +1,27 @@
+let innerOptional = (): Json? => {
+  return nil;
+};
+
+let outerJson = (x: Json) => {
+
+};
+
+
+let innerJson = (): Json => {
+  return {};
+};
+
+let outerOptional = (x: Json?) => {
+
+};
+
+
+// those are ok
+outerJson(innerJson());
+outerOptional(innerOptional());
+outerOptional(innerJson());
+
+// this is problematic as json? can be nil, and nil is not json
+outerJson(innerOptional());
+        //^^^^^^^^^^^^^^^ Expected type to be "Json", but got "Json?" instead
+
diff --git a/examples/tests/sdk_tests/resource/call.test.w b/examples/tests/sdk_tests/resource/call.test.w
index 3882c68a618..ec7b3f0d0d7 100644
--- a/examples/tests/sdk_tests/resource/call.test.w
+++ b/examples/tests/sdk_tests/resource/call.test.w
@@ -119,11 +119,11 @@ class MyResource {
   }
   pub inflight methodOptNum1(arg: num?): num? {
     // TODO: need a way to convert from Json to num?
-    return unsafeCast(this.backend.call("methodOptNum1", Json [arg]));
+    return unsafeCast(this.backend.call("methodOptNum1", Json [arg ?? 0]));
   }
   pub inflight methodOptNum2(arg: num?): num? {
     // TODO: need a way to convert from Json to num?
-    return unsafeCast(this.backend.call("methodOptNum2", Json [arg]));
+    return unsafeCast(this.backend.call("methodOptNum2", Json [arg ?? 0]));
   }
   pub inflight methodJson(arg: Json): Json {
     return this.backend.call("methodJson", [arg]);
diff --git a/examples/tests/sdk_tests/table/get.test.w b/examples/tests/sdk_tests/table/get.test.w
index ff6046a93b9..6090ddef1a1 100644
--- a/examples/tests/sdk_tests/table/get.test.w
+++ b/examples/tests/sdk_tests/table/get.test.w
@@ -32,7 +32,7 @@ test "get" {
     table.get(NON_EXISTENT_KEY);
   });
 
-  let var result: Json = table.tryGet(VALID_KEY);
-  assert(result.get(COLUMN_NAME) == COLUMN_VALUE);
+  let var result: Json? = table.tryGet(VALID_KEY);
+  assert(result?.get(COLUMN_NAME) == COLUMN_VALUE);
   assert(table.tryGet(NON_EXISTENT_KEY) == nil);
 }
diff --git a/libs/wingc/src/type_check.rs b/libs/wingc/src/type_check.rs
index 38227371819..4dd1777ed2e 100644
--- a/libs/wingc/src/type_check.rs
+++ b/libs/wingc/src/type_check.rs
@@ -1253,7 +1253,6 @@ impl TypeRef {
 			Type::Inferred(..) => true,
 			Type::Array(v) => v.is_json_legal_value(),
 			Type::Map(v) => v.is_json_legal_value(),
-			Type::Optional(v) => v.is_json_legal_value(),
 			Type::Struct(ref s) => {
 				for (_, t) in s.fields(true) {
 					if !t.is_json_legal_value() {
@@ -3489,7 +3488,11 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] });
 			));
 		}
 
-		if matches!(**return_type.maybe_unwrap_option(), Type::Json(None) | Type::MutJson) {
+		if matches!(**return_type.maybe_unwrap_option(), Type::Json(None) | Type::MutJson)
+			&& !matches!(
+				**first_expected_type.maybe_unwrap_option(),
+				Type::Json(None) | Type::MutJson
+			) {
 			// known json data is statically known
 			hints.push(format!(
 				"use {first_expected_type}.fromJson() to convert dynamic Json\""
diff --git a/tools/hangar/__snapshots__/invalid.ts.snap b/tools/hangar/__snapshots__/invalid.ts.snap
index 88d90bf3025..ea6d1e63380 100644
--- a/tools/hangar/__snapshots__/invalid.ts.snap
+++ b/tools/hangar/__snapshots__/invalid.ts.snap
@@ -3135,6 +3135,21 @@ error: \\"Bucket\\" is not a legal JSON value
 
 
 
+Tests 1 failed (1)
+Snapshots 1 skipped
+Test Files 1 failed (1)
+Duration <DURATION>"
+`;
+
+exports[`json_is_not_nil.test.w 1`] = `
+"error: Expected type to be \\"Json\\", but got \\"Json?\\" instead
+   --> ../../../examples/tests/invalid/json_is_not_nil.test.w:25:11
+   |
+25 | outerJson(innerOptional());
+   |           ^^^^^^^^^^^^^^^
+
+
+
 Tests 1 failed (1)
 Snapshots 1 skipped
 Test Files 1 failed (1)