Skip to content

Chore: Also list native files with clang-tidy on android #9164

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wisniewskij wants to merge 5 commits into
base: main
Choose a base branch
from
+18 −14
Open

Chore: Also list native files with clang-tidy on android #9164

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
33ec4ca
Update packages
wisniewskij Mar 23, 2026
74769d1
Merge branch 'main' into @wisniewskij/lint-everything-on-android
wisniewskij Mar 23, 2026
f72be09
Podfiles
wisniewskij Mar 23, 2026
7bc3720
Fix linting of worklets
wisniewskij Mar 23, 2026
42e0d85
Fix android linting
wisniewskij Mar 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions apps/fabric-example/ios/Podfile.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion apps/macos-example/macos/Podfile.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions apps/tvos-example/ios/Podfile.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/react-native-reanimated/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"lint:js": "eslint src __tests__ __typetests__ && yarn prettier --check src __tests__ __typetests__",
"lint:android": "../../scripts/validate-android.sh && ./android/gradlew -p android spotlessCheck -q && ../../scripts/cpplint.sh android/src && yarn format:android:cpp --dry-run -Werror && yarn lint:cmake",
"lint:common": "../../scripts/validate-common.sh && ../../scripts/cpplint.sh Common && yarn format:common --dry-run -Werror",
"lint:clang-tidy": "../../scripts/clang-tidy-lint.sh \"Common\"",
Copy link
Collaborator

@tjzel tjzel Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will lint both android and apple files during a single run and we don't want that as compile_commands.json are only per single platform - this should be split into several commands.

Copy link
Collaborator

@tjzel tjzel Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, please make lint-clang-tidy GitHub action triggerable by workflow_dispatch so we could manually dispatch the action to test this PR.

Copy link
Contributor Author

@wisniewskij wisniewskij Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure that's not the case since run-clang-tidy lints strictly only files that are in the compilation database, which is generated per given platform

Copy link
Collaborator

@tjzel tjzel Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please verify it somehow 🙏

"lint:clang-tidy": "../../scripts/clang-tidy-lint.sh \".\"",
"lint:apple": "../../scripts/validate-apple.sh && yarn format:apple --dry-run -Werror",
"lint:cmake": "find ./android -type d \\( -name build -o -name .cxx \\) -prune -o -type f -name 'CMakeLists.txt' -print | xargs ../../scripts/lint-cmake.sh",
"format": "yarn format:js && yarn format:apple && yarn format:android && yarn format:common",
Expand Down
9 changes: 6 additions & 3 deletions packages/react-native-worklets/android/src/main/cpp/worklets/android/AndroidUIScheduler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#include <worklets/android/AndroidUIScheduler.h>

#include <utility>

namespace worklets {

using namespace facebook;
Expand All @@ -11,7 +13,7 @@ class UISchedulerWrapper : public UIScheduler {

public:
explicit UISchedulerWrapper(jni::global_ref<AndroidUIScheduler::javaobject> androidUiScheduler)
: androidUiScheduler_(androidUiScheduler) {}
: androidUiScheduler_(std::move(androidUiScheduler)) {}

void scheduleOnUI(std::function<void()> job) override {
UIScheduler::scheduleOnUI(job);
Expand All @@ -22,10 +24,11 @@ class UISchedulerWrapper : public UIScheduler {
}
};

AndroidUIScheduler::AndroidUIScheduler(jni::alias_ref<AndroidUIScheduler::javaobject> jThis)
AndroidUIScheduler::AndroidUIScheduler(const jni::alias_ref<AndroidUIScheduler::jhybridobject> &jThis)
: javaPart_(jni::make_global(jThis)), uiScheduler_(std::make_shared<UISchedulerWrapper>(jni::make_global(jThis))) {}

jni::local_ref<AndroidUIScheduler::jhybriddata> AndroidUIScheduler::initHybrid(jni::alias_ref<jhybridobject> jThis) {
jni::local_ref<AndroidUIScheduler::jhybriddata> AndroidUIScheduler::initHybrid(
jni::alias_ref<jhybridobject> jThis) { // NOLINT //(performance-unnecessary-value-param)
return makeCxxInstance(jThis);
}

Expand Down
5 changes: 3 additions & 2 deletions packages/react-native-worklets/android/src/main/cpp/worklets/android/AndroidUIScheduler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ using namespace worklets;
class AndroidUIScheduler : public jni::HybridClass<AndroidUIScheduler> {
public:
static auto constexpr kJavaDescriptor = "Lcom/swmansion/worklets/AndroidUIScheduler;";
static jni::local_ref<jhybriddata> initHybrid(jni::alias_ref<jhybridobject> jThis);
static jni::local_ref<jhybriddata> initHybrid(
jni::alias_ref<jhybridobject> jThis); // NOLINT //(performance-unnecessary-value-param)
static void registerNatives();

std::shared_ptr<UIScheduler> getUIScheduler() {
Expand All @@ -34,7 +35,7 @@ class AndroidUIScheduler : public jni::HybridClass<AndroidUIScheduler> {
jni::global_ref<AndroidUIScheduler::javaobject> javaPart_;
std::shared_ptr<UIScheduler> uiScheduler_;

explicit AndroidUIScheduler(jni::alias_ref<AndroidUIScheduler::jhybridobject> jThis);
explicit AndroidUIScheduler(const jni::alias_ref<AndroidUIScheduler::jhybridobject> &jThis);
};

} // namespace worklets
2 changes: 1 addition & 1 deletion packages/react-native-worklets/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"lint:apple": "../../scripts/validate-apple.sh && yarn format:apple --dry-run -Werror",
"lint:common": "../../scripts/validate-common.sh && ../../scripts/cpplint.sh Common && yarn format:common --dry-run -Werror",
"lint:cmake": "find ./android -type d \\( -name build -o -name .cxx \\) -prune -o -type f -name 'CMakeLists.txt' -print | xargs ../../scripts/lint-cmake.sh",
"lint:clang-tidy": "../../scripts/clang-tidy-lint.sh \"Common\"",
"lint:clang-tidy": "../../scripts/clang-tidy-lint.sh \".\"",
"lint:js": "eslint src && yarn prettier --check src",
"lint:plugin": "yarn workspace babel-plugin-worklets lint",
"test": "jest",
Expand Down
Loading