Skip to content

Commit 3bac866

Browse files
davidpichardiemeta-codesync[bot]
davidpichardie
authored and
meta-codesync[bot]
committed
[semdiff][B006][5/5] wire --semdiff-b006-migration CLI and integration test
Summary: Expose B006 migration checking on the command line. - Add the --semdiff-b006-migration flag (Config). - Add Semdiff.semdiff_b006_textual: for each procedure, extract the parameter defaults from both modules and call TextualPegDiff.check_b006_migration; report a mismatch if any pair is not accepted. - Wire it into the semdiff command (factoring the shared textual- migration plumbing with the B007 path). Add the python_semdiff-b006 direct test: - equal_list_default / equal_list_default_used / equal_dict_default: the codemod is accepted (no issue reported). - different_wrong_literal ([] replaced by {}) and different_body: rejected (SEMDIFF MISMATCH). Usage: infer semdiff --semdiff-b006-migration \ --semdiff-previous old.py --semdiff-current new.py -o infer-out Reviewed By: dulmarod Differential Revision: D106821143 fbshipit-source-id: 79843441f50b61a8695c4c81350417cc9c515c8e
1 parent afceabe commit 3bac866

21 files changed

Lines changed: 209 additions & 3 deletions

‎Makefile‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,7 @@ DIRECT_TESTS += \
254254
python_export-textual \
255255
python_pulse \
256256
python_semdiff \
257+
python_semdiff-b006 \
257258
python_semdiff-b007 \
258259
python_semdiff-demo \
259260

‎infer/man/man1/infer-full.txt‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2194,6 +2194,15 @@ OPTIONS
21942194
(Conversely: --no-self-in-block-only)
21952195
See also infer-analyze(1).
21962196

2197+
--semdiff-b006-migration
2198+
Activates: Check B006 migration: verify that the current program
2199+
is a valid rewrite of the previous program where a mutable default
2200+
argument has been replaced by None plus an "if p is None: p =
2201+
<literal>" guard. Equivalence holds under the assumption that
2202+
explicitly-passed arguments are not None. (Conversely:
2203+
--no-semdiff-b006-migration)
2204+
See also infer-semdiff(1).
2205+
21972206
--semdiff-b007-migration
21982207
Activates: Check B007 migration: verify that the current program
21992208
is a valid simplification of the previous program (remove unused

‎infer/man/man1/infer-semdiff.txt‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,14 @@ OPTIONS
4646
Show this manual with all internal options in the INTERNAL OPTIONS
4747
section
4848

49+
--semdiff-b006-migration
50+
Activates: Check B006 migration: verify that the current program
51+
is a valid rewrite of the previous program where a mutable default
52+
argument has been replaced by None plus an "if p is None: p =
53+
<literal>" guard. Equivalence holds under the assumption that
54+
explicitly-passed arguments are not None. (Conversely:
55+
--no-semdiff-b006-migration)
56+
4957
--semdiff-b007-migration
5058
Activates: Check B007 migration: verify that the current program
5159
is a valid simplification of the previous program (remove unused

‎infer/man/man1/infer.txt‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2194,6 +2194,15 @@ OPTIONS
21942194
(Conversely: --no-self-in-block-only)
21952195
See also infer-analyze(1).
21962196

2197+
--semdiff-b006-migration
2198+
Activates: Check B006 migration: verify that the current program
2199+
is a valid rewrite of the previous program where a mutable default
2200+
argument has been replaced by None plus an "if p is None: p =
2201+
<literal>" guard. Equivalence holds under the assumption that
2202+
explicitly-passed arguments are not None. (Conversely:
2203+
--no-semdiff-b006-migration)
2204+
See also infer-semdiff(1).
2205+
21972206
--semdiff-b007-migration
21982207
Activates: Check B007 migration: verify that the current program
21992208
is a valid simplification of the previous program (remove unused

‎infer/src/base/Config.ml‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3345,6 +3345,15 @@ and semdiff_experimental_eqsat_engine =
33453345
"Use experimental equality saturation rewriting engine."
33463346

33473347

3348+
and semdiff_b006_migration =
3349+
CLOpt.mk_bool ~long:"semdiff-b006-migration" ~default:false
3350+
~in_help:InferCommand.[(SemDiff, manual_generic)]
3351+
"Check B006 migration: verify that the current program is a valid rewrite of the previous \
3352+
program where a mutable default argument has been replaced by None plus an \"if p is None: p \
3353+
= <literal>\" guard. Equivalence holds under the assumption that explicitly-passed arguments \
3354+
are not None."
3355+
3356+
33483357
and semdiff_b007_migration =
33493358
CLOpt.mk_bool ~long:"semdiff-b007-migration" ~default:false
33503359
~in_help:InferCommand.[(SemDiff, manual_generic)]
@@ -4982,6 +4991,8 @@ and semdiff_test_actions = !semdiff_test_actions
49824991

49834992
and semdiff_experimental_eqsat_engine = !semdiff_experimental_eqsat_engine
49844993

4994+
and semdiff_b006_migration = !semdiff_b006_migration
4995+
49854996
and semdiff_b007_migration = !semdiff_b007_migration
49864997

49874998
and semdiff_configuration = RevList.to_list !semdiff_configuration

‎infer/src/base/Config.mli‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -832,6 +832,8 @@ val semdiff_test_show_syntax_errors : bool
832832

833833
val semdiff_experimental_eqsat_engine : bool
834834

835+
val semdiff_b006_migration : bool
836+
835837
val semdiff_b007_migration : bool
836838

837839
val semdiff_configuration : string list

‎infer/src/integration/InferCommandImplementation.ml‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -452,16 +452,19 @@ let sem_diff () =
452452
"Expected '--semdiff-current' and '--semdiff-previous', '--semdiff-test-files-index', or \
453453
'--semdiff-from-json' to be specified."
454454
| Some (previous_file, current_file), None, None ->
455-
if Config.semdiff_b007_migration then (
455+
let run_textual_migration migration_fn =
456456
let module_old = python_file_to_textual previous_file in
457457
let module_new = python_file_to_textual current_file in
458458
let debug = Config.debug_mode in
459-
let diffs = Semdiff.semdiff_b007_textual ~debug module_old module_new in
459+
let diffs = migration_fn ~debug module_old module_new in
460460
let out_path = ResultsDir.get_path SemDiff in
461461
Diff.write_json ~previous_file ~current_file ~out_path diffs ;
462462
Option.iter Config.issues_tests ~f:(fun out_path ->
463463
let json_path = ResultsDir.get_path SemDiff in
464-
Diff.write_from_json ~json_path ~out_path ) )
464+
Diff.write_from_json ~json_path ~out_path )
465+
in
466+
if Config.semdiff_b007_migration then run_textual_migration Semdiff.semdiff_b007_textual
467+
else if Config.semdiff_b006_migration then run_textual_migration Semdiff.semdiff_b006_textual
465468
else
466469
let config_files = Config.semdiff_configuration in
467470
Semdiff.semdiff ~config_files ~previous_file ~current_file ;

‎infer/src/semdiff/Semdiff.ml‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,34 @@ let semdiff_b007_textual ~debug (module_old : Textual.Module.t) (module_new : Te
101101
if all_accepted then [] else [Diff.dummy_explicit]
102102

103103

104+
let semdiff_b006_textual ~debug (module_old : Textual.Module.t) (module_new : Textual.Module.t) =
105+
let procs_old = extract_procs module_old in
106+
let procs_new = extract_procs module_new in
107+
let all_accepted =
108+
List.for_all procs_old ~f:(fun proc_old ->
109+
let name = proc_fun_name proc_old in
110+
if is_module_body name then true
111+
else
112+
match List.find procs_new ~f:(fun p -> String.equal (proc_fun_name p) name) with
113+
| None ->
114+
if debug then L.user_error "Procedure %s not found in current file@." name ;
115+
true (* procedure removed — not a migration concern *)
116+
| Some proc_new -> (
117+
let defaults_old = StructuredPeg.extract_defaults module_old proc_old in
118+
let defaults_new = StructuredPeg.extract_defaults module_new proc_new in
119+
match
120+
TextualPegDiff.check_b006_migration ~debug ~defaults_old ~defaults_new proc_old
121+
proc_new
122+
with
123+
| result ->
124+
result
125+
| exception CongruenceClosureRewrite.Rule.FuelExhausted _ ->
126+
if debug then L.user_error "Fuel exhausted for procedure %s@." name ;
127+
false ) )
128+
in
129+
if all_accepted then [] else [Diff.dummy_explicit]
130+
131+
104132
let semdiff_from_json ~config_files json_path =
105133
let debug = Config.debug_mode in
106134
let input = Semdiff_batch_j.semdiff_input_of_string (In_channel.read_all json_path) in

‎infer/src/semdiff/Semdiff.mli‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,6 @@ val semdiff : config_files:string list -> previous_file:string -> current_file:s
2222

2323
val semdiff_b007_textual : debug:bool -> Textual.Module.t -> Textual.Module.t -> Diff.explicit list
2424

25+
val semdiff_b006_textual : debug:bool -> Textual.Module.t -> Textual.Module.t -> Diff.explicit list
26+
2527
val semdiff_from_json : config_files:string list -> string -> unit

‎infer/tests/codetoanalyze/python/semdiff-b006/Makefile‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
# Copyright (c) Facebook, Inc. and its affiliates.
2+
#
3+
# This source code is licensed under the MIT license found in the
4+
# LICENSE file in the root directory of this source tree.
5+
6+
TESTS_DIR = ../../..
7+
INFER_OUT = infer-out
8+
INFERPRINT_OPTIONS = --issues-tests
9+
10+
include $(TESTS_DIR)/base.make
11+
12+
SOURCES = $(sort $(wildcard previous/*.py))
13+
BASENAMES = $(notdir $(SOURCES))
14+
15+
default: test
16+
17+
# Run semdiff B006 migration check on all file pairs, accumulating results
18+
issues.exp.test: $(SOURCES) $(wildcard current/*.py) $(INFER_BIN)
19+
$(QUIET)mkdir -p $(INFER_OUT)
20+
$(QUIET)rm -f $@
21+
$(QUIET)for f in $(BASENAMES); do \
22+
$(INFER_BIN) semdiff --quiet --no-progress-bar \
23+
--semdiff-b006-migration \
24+
--semdiff-previous previous/$$f \
25+
--semdiff-current current/$$f \
26+
--issues-tests $@ \
27+
-o $(INFER_OUT)/$$f.out 2>&1 || true; \
28+
done
29+
$(QUIET)sed -i 's|$(CURDIR)/||g' $@
30+
31+
.PHONY: analyze
32+
analyze: issues.exp.test
33+
34+
.PHONY: print
35+
print: issues.exp.test
36+
$(QUIET)cat $<
37+
38+
.PHONY: test
39+
test: issues.exp.test
40+
$(QUIET)cd $(TESTS_DIR) && \
41+
$(call check_no_diff,$(TEST_REL_DIR)/issues.exp,$(TEST_REL_DIR)/issues.exp.test)
42+
43+
.PHONY: replace
44+
replace: issues.exp.test
45+
cp $< issues.exp
46+
47+
.PHONY: clean
48+
clean:
49+
$(REMOVE_DIR) $(INFER_OUT) issues.exp.test

0 commit comments

Comments
 (0)