From 17905f034a0484df5c01691788d4bf47549551f0 Mon Sep 17 00:00:00 2001 From: Ilya Zorin Date: Fri, 1 Nov 2024 09:36:16 -0700 Subject: [PATCH] [hack][builder pattern] Add FP test case with not finalised builder that doesn't have any method calls Summary: We don't want to report builders that don't have any method calls. This may happen if builder finalisation is guarded by an external variable. This diffs adds the test case that highlights this problem. Reviewed By: skcho Differential Revision: D65338134 fbshipit-source-id: 4af7bbdd0d55ae9e3fbb81443d319443af2d800d --- infer/tests/codetoanalyze/hack/pulse/builder.hack | 14 ++++++++++++++ infer/tests/codetoanalyze/hack/pulse/issues.exp | 1 + 2 files changed, 15 insertions(+) diff --git a/infer/tests/codetoanalyze/hack/pulse/builder.hack b/infer/tests/codetoanalyze/hack/pulse/builder.hack index e2d0aeb5f08..21f5c6575c8 100644 --- a/infer/tests/codetoanalyze/hack/pulse/builder.hack +++ b/infer/tests/codetoanalyze/hack/pulse/builder.hack @@ -101,6 +101,20 @@ class BuilderTester { } } + public static function FP_builderWithoutCallsOk(bool $flag): void { + $changes_made = false; + $b = new MyBuilder(0); + + if ($flag) { + $b->setA(42); + $changes_made = true; + } + + if ($changes_made) { + $b->saveX(); + } + } + // now check builders recognised from the .inferconfig file public static function testConfigBad(): void { $b = new NoBuilderSuffix(); diff --git a/infer/tests/codetoanalyze/hack/pulse/issues.exp b/infer/tests/codetoanalyze/hack/pulse/issues.exp index eb8d3ce1775..c0440cb827a 100644 --- a/infer/tests/codetoanalyze/hack/pulse/issues.exp +++ b/infer/tests/codetoanalyze/hack/pulse/issues.exp @@ -28,6 +28,7 @@ booleans.hack, $root.Booleans::testBoolBad, 3, TAINT_ERROR, no_bucket, ERROR, [s booleans.hack, $root.Booleans::testNullBad, 3, TAINT_ERROR, no_bucket, ERROR, [source of the taint here: value passed as argument `#0` to `$root.Booleans::testNullBad` with kind `Simple`,flows to this sink: value passed as argument `#0` to `$root.Level1::taintSink` with kind `Simple`], source: $root.Booleans::testNullBad, sink: $root.Level1::taintSink, tainted expression: $sc booleans.hack, $root.Booleans::testNotNullBad, 3, TAINT_ERROR, no_bucket, ERROR, [source of the taint here: value passed as argument `#0` to `$root.Booleans::testNotNullBad` with kind `Simple`,flows to this sink: value passed as argument `#0` to `$root.Level1::taintSink` with kind `Simple`], source: $root.Booleans::testNotNullBad, sink: $root.Level1::taintSink, tainted expression: $sc builder.hack, BuilderTester$static.builderUserBad, 3, PULSE_UNFINISHED_BUILDER, no_bucket, ERROR, [allocation part of the trace starts here,allocated by constructor `MyBuilder()` here,builder object becomes unreachable here] +builder.hack, BuilderTester$static.FP_builderWithoutCallsOk, 9, PULSE_UNFINISHED_BUILDER, no_bucket, ERROR, [allocation part of the trace starts here,allocated by constructor `MyBuilder()` here,builder object becomes unreachable here] builder.hack, BuilderTester$static.testConfigBad, 2, PULSE_UNFINISHED_BUILDER, no_bucket, ERROR, [allocation part of the trace starts here,allocated by constructor `NoBuilderSuffix()` here,builder object becomes unreachable here] builder.hack, BuilderTester$static.testConfigBad2, 3, PULSE_UNFINISHED_BUILDER, no_bucket, ERROR, [allocation part of the trace starts here,allocated by constructor `NoBuilderSuffix()` here,builder object becomes unreachable here] builder.hack, BuilderTester2$static.testCreateBad, 3, PULSE_UNFINISHED_BUILDER, no_bucket, ERROR, [allocation part of the trace starts here,when calling `BuilderTester2$static.create` here,when calling `MyBuilder$static.__factory` here,allocated by constructor `MyBuilder()` here,builder object becomes unreachable here]