Rubberduck: Refactor.Rename undoes previous edits

Created on 4 Feb 2018  路  5Comments  路  Source: rubberduck-vba/Rubberduck

Rubberduck Rubberduck version: Version 2.1.2.2825

If I make some edits and then do a refactor rename, all of the edits I made revert to the original text. The rename succeeds and is not reverted on the next refactor.rename.

bug critical feature-inspection-quickfixes feature-refactorings parse-tree-processing

Most helpful comment

The RubberduckParserState has a method IsNewOrModified that can be used to check whether the content of a module has been changed. This had a caching bug that I fixed in #3735.

All 5 comments

Thank you for reporting this. This is most likely caused by a caching issue that makes the parsing engine think that it does not have to reparse the module before renaming.

A fix for the most probable cause is included in PR #3735, which has not been merged so far.

Note that making some edits and then using any Rubberduck code-rewriting feature, will produce this result.

  1. RD parses VBA code
  2. User makes code changes
  3. User requests a reparse (this is the part that should be automated)
  4. RD makes code changes -> nothing lost.

However:

  1. RD parses VBA code
  2. User makes code changes
  3. RD makes code changes -> user changes since last parse were not in the rewriter's token stream: rewritten module doesn't include them, changes are lost.

IOW this is lower-level than refactor/rename; it's the parser state cache and module rewriters that need to get involved.

In simpler terms - code-rewriting features currently assume that the parser state is up-to-date, instead of requesting a reparse before proceeding.

We could make quickfixes and refactorings' base classes request a reparse, however that reparse should be blocking/synchronous, so as to actively prevent any further code changes during the reparse.

@MDoerner how did #3735 address this?

The commands' CanExecute implementations need to get a ContentHash diff for the current/active module from the parser state, however it would be rather inefficient to hash a module's content once for every single RD command that needs to check whether it should be enabled or not... and caching the content hash means we need to somehow invalidate that cache at some point... I don't see a definitive fix for this, at least until the code panes fire some Changed event.

The RubberduckParserState has a method IsNewOrModified that can be used to check whether the content of a module has been changed. This had a caching bug that I fixed in #3735.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

connerk picture connerk  路  61Comments

UnhandledCoder picture UnhandledCoder  路  32Comments

daFreeMan picture daFreeMan  路  25Comments

A9G-Data-Droid picture A9G-Data-Droid  路  21Comments

ishita799 picture ishita799  路  28Comments