Split move detection out into a new MoveDetector class.

Review Request #9698 — Created Feb. 24, 2018 and updated


Review Board


Move detection has become a lot more complicated since it was first
introduced, and over time has become a significant portion of
DiffOpcodeGenerator's code. With some upcoming changes, it's going to
grow in complexity and will need to store some state on the instance
during processing. Rather than complicate the opcode generator even
more, I've split out the code into a new class.

The new MoveDetector take a differ and opcodes and runs through them,
running the move detection algorithm. It can be used without the rest of
DiffOpcodeGenerator now, which is handy for unit testing and might be
useful down the road for other purposes. We may also be able to rework
this down the road to detect moves across files, which would require
separating the detector from the opcode generator. For now, it keep the
code a lot more self-contained and organized and will open the doors to
some of the restructuring work I have planned to further improve the
detection of moved ranges.

The new code is the same as the old, except it's gained docstrings and
some clarifications, and has reduced the amount of attribute lookups
needed while iterating through files. Algorithmically, nothing has

DiffOpcodeGenerator has also gained a new enable_move_detection
option, which gives callers control over whether to perform move
detection. I plan to introduce similar options for other parts of the
opcode generation process in future changes.

Unit tests pass.

Viewed diffs using move detection. The new diffs exactly matched the
old ones.

Description From Last Updated

E303 too many blank lines (3)

Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.


  1. Hold off on reviewing this. I'm going to put up a change tomorrow that I'm going to base parts of this on.