This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Improve the output of rename tests where there are failures.
ClosedPublic

Authored by hokein on Nov 6 2019, 6:13 AM.

Details

Summary

Previously, we match ranges, which is hard to spot the difference.
Now, we diff the code after rename against the expected result, it
produces much nicer output.

Diff Detail

Event Timeline

hokein created this revision.Nov 6 2019, 6:13 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 6 2019, 6:13 AM
ilya-biryukov accepted this revision.Nov 6 2019, 6:24 AM

LGTM! Many thanks, the new tests are much simpler to read!

clang-tools-extra/clangd/unittests/RenameTests.cpp
31

NIT: Test.llvm::Annotations::ranges() will return ranges with offsets.
Should simplify the code.

44

NIT: worth documenting that ^ points to the start position of the rename and ranges point to the identifier that is being renamed.

44

NIT: maybe use llvm::StringRef Tests[] instead?

This revision is now accepted and ready to land.Nov 6 2019, 6:24 AM
hokein updated this revision to Diff 228054.Nov 6 2019, 6:34 AM
hokein marked 3 inline comments as done.

address review comments.

This revision was automatically updated to reflect the committed changes.