[clang] [clang-refactor] Add Matcher Edit refactoring rule (PR #123782)
Сергеев Игнатий via cfe-commits
cfe-commits at lists.llvm.org
Sun Jan 26 08:22:14 PST 2025
IgnatSergeev wrote:
> @IgnatSergeev, thanks for posting this PR!
>
> First question, where is this new code tested?
>
> Do you know the status of automated testing for this LLVM refactoring support software and the `clang-refactor` tool?
>
> According to:
>
> * https://llvm.org/docs/TestingGuide.html
>
> unit, integration and regression tests can be added under `llvm/unittests/` and `llvm/test/`. Is there a way to add tests for new code like this?
>
> I don't see any existing tests for the `clang-refactor` tool itself looking there:
>
> ```
> $ cd llvm-project/
>
> $ git log-short --name-status -1 HEAD
> 3d08fa25824c "[libc++] Another _LIBCPP_NODEBUG fix"
> Author: Louis Dionne <ldionne.2 at gmail.com>
> Date: Mon Jan 20 14:15:50 2025 -0500 (25 hours ago)
>
> $ cd llvm/
>
> M libcxx/include/future
>
> $ find unittests test -type f -exec grep -nHi clang-refactor {} \;
> [empty]
> ```
>
> But do see some hits under `clang/test/Refactor/`:
>
> ```
> $ find . -type f -exec grep -nHi 'clang-refactor' {} \;
>
> ...
> ./clang/test/Refactor/Extract/ExtractExprIntoFunction.cpp:1:// RUN: clang-refactor extract -selection=test:%s %s -- -std=c++14 2>&1 | grep -v CHECK | FileCheck %s
> ./clang/test/Refactor/Extract/ExtractionSemicolonPolicy.cpp:1:// RUN: clang-refactor extract -selection=test:%s %s -- -std=c++11 -fcxx-exceptions | grep -v CHECK | FileCheck %s
> ./clang/test/Refactor/Extract/ExtractionSemicolonPolicy.m:1:// RUN: clang-refactor extract -selection=test:%s %s -- 2>&1 | grep -v CHECK | FileCheck %s
> ./clang/test/Refactor/Extract/FromMethodToFunction.cpp:1:// RUN: clang-refactor extract -selection=test:%s %s -- -std=c++11 2>&1 | grep -v CHECK | FileCheck --check-prefixes=CHECK,CHECK-INNER %s
> ./clang/test/Refactor/Extract/FromMethodToFunction.cpp:2:// RUN: clang-refactor extract -selection=test:%s %s -- -std=c++11 -DMULTIPLE 2>&1 | grep -v CHECK | FileCheck --check-prefixes=CHECK,CHECK-OUTER %s
> ./clang/test/Refactor/Extract/ObjCProperty.m:1:// RUN: clang-refactor extract -selection=test:%s %s -- 2>&1 | grep -v CHECK | FileCheck %s
> ./clang/test/Refactor/LocalRename/BuiltinOffsetof.cpp:1:// RUN: clang-refactor local-rename -selection=test:%s -new-name=bar %s -- | grep -v CHECK | FileCheck %s
> ./clang/test/Refactor/LocalRename/Field.cpp:1:// RUN: clang-refactor local-rename -selection=test:%s -new-name=Bar %s -- | grep -v CHECK | FileCheck %s
> ./clang/test/Refactor/LocalRename/NoSymbolSelectedError.cpp:1:// RUN: not clang-refactor local-rename -selection=%s:4:1 -new-name=Bar %s -- 2>&1 | grep -v CHECK | FileCheck %s
> ./clang/test/Refactor/LocalRename/NoSymbolSelectedError.cpp:2:// RUN: clang-refactor local-rename -selection=test:%s -new-name=Bar %s -- 2>&1 | grep -v CHECK | FileCheck --check-prefix=TESTCHECK %s
> ./clang/test/Refactor/LocalRename/QualifiedRename.cpp:1:// RUN: clang-refactor local-rename -old-qualified-name="foo::A" -new-qualified-name="bar::B" %s -- -std=c++11 2>&1 | grep -v CHECK | FileCheck %s
> ./clang/test/Refactor/tool-apply-replacements.cpp:2:// RUN: clang-refactor local-rename -selection=%t.cpp:7:7 -new-name=test %t.cpp -- | FileCheck %s
> ./clang/test/Refactor/tool-apply-replacements.cpp:3:// RUN: clang-refactor local-rename -selection=%t.cpp:7:7-7:15 -new-name=test %t.cpp -- | FileCheck %s
> ./clang/test/Refactor/tool-apply-replacements.cpp:4:// RUN: clang-refactor local-rename -i -selection=%t.cpp:7:7 -new-name=test %t.cpp --
> ./clang/test/Refactor/tool-common-options.c:1:// RUN: not clang-refactor 2>&1 | FileCheck --check-prefix=MISSING_ACTION %s
> ./clang/test/Refactor/tool-selection-option.c:3:// RUN: clang-refactor local-rename -selection=%t.cp.c:6:5 -new-name=test -v %t.cp.c -- | FileCheck --check-prefix=CHECK1 %s
> ./clang/test/Refactor/tool-selection-option.c:4:// RUN: clang-refactor local-rename -selection=%t.cp.c:6:5-6:9 -new-name=test -v %t.cp.c -- | FileCheck --check-prefix=CHECK2 %s
> ./clang/test/Refactor/tool-selection-option.c:14:// RUN: not clang-refactor local-rename -selection=%s:6:5 -new-name=test -v %t.cp.c -- 2>&1 | FileCheck --check-prefix=CHECK-FILE-ERR %s
> ./clang/test/Refactor/tool-test-support.c:1:// RUN: clang-refactor local-rename -selection=test:%s -new-name=test -v %s -- | FileCheck %s
> ...
> ```
>
> There is no mention at all about tests under `clang/test/Refactor/` in https://llvm.org/docs/TestingGuide.html (or anywhere in a Google search that I can find).
>
> Looking at the GitHub Actions defined for this GitHub repo at:
>
> * https://github.com/llvm/llvm-project/actions
>
> and it is not clear which of these GHAs run run actual tests. (Looks like most of those GHAs are just doing some PR automation and not really testing anything.)
Speaking about testing infrastructure, as you pointed out clang-refactor has tests under clang/test (and actually clang/unittests), tests for clang and tools that are based on it (including clang-refactor) are separated from llvm tests. They could be run with these cmake targets: check-clang and check-clang-unit.
As for GitHub Actions, I think build and test CI should be triggered manually by a maintainer.
Speaking about test's content, as much as I understand, clang-refactor tool has tests for:
- complex CLI options (clang/test/Refactor/tool-selection-option.c, ...)
- every Refactoring Action (clang/test/Refactor/LocalRename and clang/test/Refactor/Extract)
It has some unit tests, but they mostly test ability to create Refactoring Rules, rather than test existing ones (clang/unittests/Tooling/RefactoringActionRulesTest.cpp)
CLI options are tested through actions, that use them, with requirement of specific test support in the tool itself (clang/tools/clang-refactor/TestSupport.cpp).
In current state, this PR adds new option and Refactoring Rules, but has no new actions (so there is no way to test anything).
But as I mentioned in closed PR, I have some dummy actions, that are based on new Refactoring Rules, with tests covering them, and test support for new CLI option.
I could add every piece of code related to those actions and tests here, and later remove them if needed.
P. S. I'm using some Refactoring Engine terms here, if you are not sure, I recommend reading these docs https://clang.llvm.org/docs/RefactoringEngine.html
https://github.com/llvm/llvm-project/pull/123782
More information about the cfe-commits
mailing list