Updated visualization of Inline Variable refactoring

[anchouls]

Used subExpressionMappings instead of references

• In 723ab1cf (Arend) commit: Before:

  

  After:

  

• In 25265e6a commit subExpressionMappings is empty

• In 7beee3d5 commit subExpressionMappings and references are empty, so the diff window doesn't open

[onewhl]

@anchouls @tsantalis I'm not sure that Inline Variable refactoring was performed in the commit. In my view, it more looks like a developer just removed one variable and used another one instead. I think Inline Variable means that a developer removes variable declaration and replaces its occurrences with its definition.

In this example, developer removed the variable callCategory and used variable definitionCallGraph instead. Variable definitionCallGraph was introduced some time ago and wasn't changed in this commit.

Please, correct me if I misunderstood something.

[onewhl]

@anchouls visualization looks cool, it's much clearer now! I only don't understand why subExpressionMappings is empty sometimes and what we should show instead. Showing empty diff is not an option.

[tsantalis]

@onewhl @anchouls https://github.com/JetBrains/Arend/commit/723ab1cf#diff-2ffe116f804e70f3a2c81d6ad4d3f6b47ca9a936dbf6eca727f2300279cda03eL410

This is a correct Inline Variable refactoring. All references to variable callCategory have been replaced with definitionCallGraph, which was an argument in the initializer of callCategory new DefinitionCallGraph(definitionCallGraph)

We can detect a refactoring, even if part of the initializer is inlined. This is quite common in practice. Not all Extract/Inline Variable refactorings are exact.

[onewhl]

@tsantalis thank you for the clarifications!

[tsantalis]

@onewhl @anchouls https://github.com/JetBrains/Arend/commit/7beee3d5#diff-772a0a3a587a50ea35bcc88bcaf6de77c88e51d771227e3d0e811526efd6cc6dL1407

This is a complex Inline Variable, as it is done indirectly. I am impressed that RefactoringMiner found it!!

The skipFromBoxing variable is inlined at myOnlySolveVars |= skipFromBoxing But skipFromBoxing is initialized with typed.getFieldKind()

and in turn typed is initialized with (SigmaTypedDependentLink)link.getNextTyped(null)

So, this is kind of the mixed result from two variables being inlined.

The inlined expression is ((SigmaTypedDependentLink) link.getNextTyped(null)).isProperty() And on top of that we have a method call rename getFieldKind() to isProperty()

This is a mind-blowing case. There are 3 refactorings (2X Inline Variable + 1X Rename Method) on top of each other.

[tsantalis]

@onewhl @anchouls

This is how we visualize it in AST diff There are no references, because we couldn't match the following statements

myOnlySolveVars |= skipFromBoxing; with myOnlySolveVars |= skipBoxed && ((SigmaTypedDependentLink) link.getNextTyped(null)).isProperty();

In recent versions, RefactoringMiner can detect Inline/Extract Variable (even) on non-mapped statements.

[tsantalis]

@onewhl @anchouls https://github.com/JetBrains/Arend/commit/25265e6a#diff-76dc783ca6289a38c4e1df8486846f50bbc3b924bd8671f6a959fdd280ec3cfdL1459

This case seems like a False Positive. Would you mind to report an issue at RefactoringMiner GitHub repo?

There is an Extract Method happening, but the code related to variables coercingField and isForced seems to be deleted, rather than inlined.

[anchouls]

https://github.com/tsantalis/RefactoringMiner/issues/416