From 4a1d8208647ec3f7cbccc4774c4bd7ae1e5ff1e8 Mon Sep 17 00:00:00 2001 From: Alexander McCord Date: Mon, 24 Mar 2025 13:54:53 -0700 Subject: [PATCH 1/3] chmod a-w a property of the discriminant type --- Analysis/src/ConstraintGenerator.cpp | 20 +++++++++++++------ tests/TypeInfer.refinements.test.cpp | 29 +++++++++++++++++++++++++++- 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/Analysis/src/ConstraintGenerator.cpp b/Analysis/src/ConstraintGenerator.cpp index 1972d559..f880f681 100644 --- a/Analysis/src/ConstraintGenerator.cpp +++ b/Analysis/src/ConstraintGenerator.cpp @@ -45,6 +45,7 @@ LUAU_FASTFLAGVARIABLE(LuauInferLocalTypesInMultipleAssignments) LUAU_FASTFLAGVARIABLE(LuauDoNotLeakNilInRefinement) LUAU_FASTFLAGVARIABLE(LuauExtraFollows) LUAU_FASTFLAG(LuauUserTypeFunTypecheck) +LUAU_FASTFLAGVARIABLE(LuauRefineJustTheReadProperty) namespace Luau { @@ -529,11 +530,18 @@ void ConstraintGenerator::computeRefinement( TypeId nextDiscriminantTy = arena->addType(TableType{}); NotNull table{getMutable(nextDiscriminantTy)}; - // When we fully support read-write properties (i.e. when we allow properties with - // completely disparate read and write types), then the following property can be - // set to read-only since refinements only tell us about what we read. This cannot - // be allowed yet though because it causes read and write types to diverge. - table->props[*key->propName] = Property::rw(discriminantTy); + if (FFlag::LuauRefineJustTheReadProperty) + { + table->props[*key->propName] = Property::readonly(discriminantTy); + } + else + { + // When we fully support read-write properties (i.e. when we allow properties with + // completely disparate read and write types), then the following property can be + // set to read-only since refinements only tell us about what we read. This cannot + // be allowed yet though because it causes read and write types to diverge. + table->props[*key->propName] = Property::rw(discriminantTy); + } table->scope = scope.get(); table->state = TableState::Sealed; @@ -547,7 +555,7 @@ void ConstraintGenerator::computeRefinement( refis->get(proposition->key->def)->shouldAppendNilType = (sense || !eq) && containsSubscriptedDefinition(proposition->key->def) && !proposition->implicitFromCall; } - else + else { refis->get(proposition->key->def)->shouldAppendNilType = (sense || !eq) && containsSubscriptedDefinition(proposition->key->def); } diff --git a/tests/TypeInfer.refinements.test.cpp b/tests/TypeInfer.refinements.test.cpp index 270707a5..99af0891 100644 --- a/tests/TypeInfer.refinements.test.cpp +++ b/tests/TypeInfer.refinements.test.cpp @@ -14,6 +14,7 @@ LUAU_FASTFLAG(LuauIntersectNotNil) LUAU_FASTFLAG(LuauSkipNoRefineDuringRefinement) LUAU_FASTFLAG(LuauFunctionCallsAreNotNilable) LUAU_FASTFLAG(LuauDoNotLeakNilInRefinement) +LUAU_FASTFLAG(LuauRefineJustTheReadProperty) using namespace Luau; @@ -2538,7 +2539,6 @@ TEST_CASE_FIXTURE(BuiltinsFixture, "function_calls_are_not_nillable") return nil end )")); - } TEST_CASE_FIXTURE(BuiltinsFixture, "oss_1528_method_calls_are_not_nillable") @@ -2562,4 +2562,31 @@ TEST_CASE_FIXTURE(BuiltinsFixture, "oss_1528_method_calls_are_not_nillable") )")); } +TEST_CASE_FIXTURE(Fixture, "refine_just_the_read_property") +{ + ScopedFastFlag sff[]{ + {FFlag::LuauSolverV2, true}, + {FFlag::LuauRefineJustTheReadProperty, true}, + }; + + CheckResult result = check(R"( + type Foo = { p: boolean } + + function f(x: Foo) + if x.p == true then return end + + x.p = true + x.p = false + end + )"); + + LUAU_REQUIRE_NO_ERRORS(result); + + // The first check is corrrect because it reflects the state of the program by that point. + // The second check is not. It fails to account for transitive typestate change from the + // previous statement. + CHECK_EQ("Foo & { read p: ~true }", toString(requireTypeAtPosition({6, 12}))); + CHECK_EQ("Foo & { read p: ~true }", toString(requireTypeAtPosition({7, 12}))); +} + TEST_SUITE_END(); From 94e5637796518bfee1d87c6e8338c9055ad58149 Mon Sep 17 00:00:00 2001 From: Alexander McCord Date: Mon, 24 Mar 2025 14:38:25 -0700 Subject: [PATCH 2/3] New keyboard. --- tests/TypeInfer.refinements.test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/TypeInfer.refinements.test.cpp b/tests/TypeInfer.refinements.test.cpp index 99af0891..55b34ece 100644 --- a/tests/TypeInfer.refinements.test.cpp +++ b/tests/TypeInfer.refinements.test.cpp @@ -2582,7 +2582,7 @@ TEST_CASE_FIXTURE(Fixture, "refine_just_the_read_property") LUAU_REQUIRE_NO_ERRORS(result); - // The first check is corrrect because it reflects the state of the program by that point. + // The first check is correct because it reflects the state of the program by that point. // The second check is not. It fails to account for transitive typestate change from the // previous statement. CHECK_EQ("Foo & { read p: ~true }", toString(requireTypeAtPosition({6, 12}))); From 86ecc212af832ea964b7843fc90aad9de06f72e4 Mon Sep 17 00:00:00 2001 From: Alexander McCord Date: Mon, 24 Mar 2025 18:39:04 -0700 Subject: [PATCH 3/3] Another test. --- tests/TypeInfer.refinements.test.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/TypeInfer.refinements.test.cpp b/tests/TypeInfer.refinements.test.cpp index 55b34ece..f3d1a09d 100644 --- a/tests/TypeInfer.refinements.test.cpp +++ b/tests/TypeInfer.refinements.test.cpp @@ -2589,4 +2589,22 @@ TEST_CASE_FIXTURE(Fixture, "refine_just_the_read_property") CHECK_EQ("Foo & { read p: ~true }", toString(requireTypeAtPosition({7, 12}))); } +TEST_CASE_FIXTURE(Fixture, "refine_just_the_read_property_typestate") +{ + ScopedFastFlag sff[]{ + {FFlag::LuauSolverV2, true}, + {FFlag::LuauRefineJustTheReadProperty, true}, + }; + + CheckResult result = check(R"( + type Foo = { p: {}? } + + function f(x: Foo) + if not x.p then x.p = {} end + end + )"); + + LUAU_REQUIRE_NO_ERRORS(result); +} + TEST_SUITE_END();