From f9899eb73f7014fee49e25df78138112246ac26c Mon Sep 17 00:00:00 2001 From: Gold856 <117957790+Gold856@users.noreply.github.com> Date: Sun, 28 Sep 2025 01:48:00 -0400 Subject: [PATCH 01/10] [wpimath] Fix odd header path in BangBangController (#8261) --- wpimath/src/main/native/cpp/controller/BangBangController.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wpimath/src/main/native/cpp/controller/BangBangController.cpp b/wpimath/src/main/native/cpp/controller/BangBangController.cpp index 7c27f4e574..56cd041813 100644 --- a/wpimath/src/main/native/cpp/controller/BangBangController.cpp +++ b/wpimath/src/main/native/cpp/controller/BangBangController.cpp @@ -2,7 +2,7 @@ // Open Source Software; you can modify and/or share it under the terms of // the WPILib BSD license file in the root directory of this project. -#include "../../include/frc/controller/BangBangController.h" +#include "frc/controller/BangBangController.h" #include From 0277759d44b99445bdc40bae996938a52beec245 Mon Sep 17 00:00:00 2001 From: Gold856 <117957790+Gold856@users.noreply.github.com> Date: Sun, 28 Sep 2025 19:00:35 -0400 Subject: [PATCH 02/10] [wpiunits] Remove redundant if statement and inaccurate comment (#8262) --- .../main/java/edu/wpi/first/units/Measure.java | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/wpiunits/src/main/java/edu/wpi/first/units/Measure.java b/wpiunits/src/main/java/edu/wpi/first/units/Measure.java index 6a436194b7..46b242b74a 100644 --- a/wpiunits/src/main/java/edu/wpi/first/units/Measure.java +++ b/wpiunits/src/main/java/edu/wpi/first/units/Measure.java @@ -665,17 +665,10 @@ public interface Measure extends Comparable> { return unit().ofBaseUnits(baseUnitResult); } - if (unit() instanceof DimensionlessUnit) { - // Numerator is a dimensionless - if (divisor.unit() instanceof PerUnit ratio) { - // Dividing by a ratio, return its reciprocal scaled by this - return ratio.reciprocal().ofBaseUnits(baseUnitResult); - } - if (divisor.unit() instanceof PerUnit ratio) { - // Dividing by a Per, return its reciprocal velocity scaled by this - // Note: Per.reciprocal() is coded to return a Velocity - return ratio.reciprocal().ofBaseUnits(baseUnitResult); - } + // Numerator is a dimensionless + if (unit() instanceof DimensionlessUnit && divisor.unit() instanceof PerUnit ratio) { + // Dividing by a ratio, return its reciprocal scaled by this + return ratio.reciprocal().ofBaseUnits(baseUnitResult); } if (divisor.unit() instanceof PerUnit ratio From bb5ee73e464635931073021346545f871e4736d6 Mon Sep 17 00:00:00 2001 From: Tyler Veness Date: Mon, 29 Sep 2025 17:47:17 -0700 Subject: [PATCH 03/10] [build] Build JNI and benchmarks as release by default (#8257) It doesn't make sense to benchmark debug binaries. Also, wpimath JNI performance in unit tests is drastically impacted by debug vs release. --- .github/workflows/fix_compile_commands.py | 3 +++ benchmark/build.gradle | 12 ++++++++++-- shared/config.gradle | 4 +++- shared/jni/setupBuild.gradle | 6 +++++- 4 files changed, 21 insertions(+), 4 deletions(-) diff --git a/.github/workflows/fix_compile_commands.py b/.github/workflows/fix_compile_commands.py index 82df7a8472..27bb412146 100755 --- a/.github/workflows/fix_compile_commands.py +++ b/.github/workflows/fix_compile_commands.py @@ -31,6 +31,9 @@ def main(): # Replace GCC warning argument with one Clang recognizes elif arg == "-Wno-maybe-uninitialized": out_args.append("-Wno-uninitialized") + # Skip GCC-specific warning argument + elif arg == "-Wno-error=restrict": + pass else: out_args.append(arg) diff --git a/benchmark/build.gradle b/benchmark/build.gradle index 5a22d693f8..7bc565d35c 100644 --- a/benchmark/build.gradle +++ b/benchmark/build.gradle @@ -181,7 +181,11 @@ tasks.register('deployStatic') { model { components { benchmarkCpp(NativeExecutableSpec) { - targetBuildTypes 'debug' + if (project.hasProperty('ciDebugOnly')) { + targetBuildTypes 'debug' + } else { + targetBuildTypes 'release' + } sources { cpp { source { @@ -235,7 +239,11 @@ model { } } benchmarkCppStatic(NativeExecutableSpec) { - targetBuildTypes 'debug' + if (project.hasProperty('ciDebugOnly')) { + targetBuildTypes 'debug' + } else { + targetBuildTypes 'release' + } nativeUtils.excludeBinariesFromStrip(it) sources { cpp { diff --git a/shared/config.gradle b/shared/config.gradle index 0b2d178c57..4776cfdecf 100644 --- a/shared/config.gradle +++ b/shared/config.gradle @@ -39,12 +39,14 @@ nativeUtils.platformConfigs.each { } } -// Compress debug info on Linux nativeUtils.platformConfigs.each { if (it.name.contains('linux')) { + // Compress debug info on Linux it.cppCompiler.debugArgs.add("-gz=zlib") // Make warning in OpenCV 4.10 from GCC 15 not an error it.cppCompiler.args.add("-Wno-error=overloaded-virtual") + // Make warning from Google Benchmark not an error + it.cppCompiler.args.add("-Wno-error=restrict") } } diff --git a/shared/jni/setupBuild.gradle b/shared/jni/setupBuild.gradle index 7cea9a8b84..4d175c0ab6 100644 --- a/shared/jni/setupBuild.gradle +++ b/shared/jni/setupBuild.gradle @@ -148,7 +148,11 @@ model { // By default, a development executable will be generated. This is to help the case of // testing specific functionality of the library. "${nativeName}Dev"(NativeExecutableSpec) { - targetBuildTypes 'debug' + if (project.hasProperty('ciDebugOnly') || project.hasProperty('debugJNI')) { + targetBuildTypes 'debug' + } else { + targetBuildTypes 'release' + } sources { cpp { source { From 6447011bc32fa9be214ddf9fd26d823952d7e073 Mon Sep 17 00:00:00 2001 From: Gold856 <117957790+Gold856@users.noreply.github.com> Date: Mon, 29 Sep 2025 21:02:42 -0400 Subject: [PATCH 04/10] [ci] Consolidate docs jobs (#7910) We build docs in three different places, which is annoying to deal with, and it means we build docs two more times than necessary. Now, docs are built just once in the main Gradle workflow, with warnings promoted to errors, eliminating the need for the separate job in lint-format.yml. The uploaded docs artifact is then unpacked and commited to the GitHub Pages repo like normal. --- .github/workflows/documentation.yml | 67 ------------- .github/workflows/gradle.yml | 64 ++++++++++++- .github/workflows/lint-format.yml | 15 --- docs/build.gradle | 47 ---------- .../0001-Remove-version-from-namespace.patch | 2 +- .../0002-Make-serializer-public.patch | 2 +- ...ke-dump_escaped-take-std-string_view.patch | 2 +- .../0004-Add-llvm-stream-support.patch | 2 +- .../0005-Fix-Doxygen-warnings.patch | 93 +++++++++++++++++++ .../main/native/include/wpinet/uv/FsEvent.h | 2 +- .../native/include/wpinet/uv/GetNameInfo.h | 1 - .../main/native/include/wpinet/uv/Stream.h | 1 - .../src/main/native/include/wpinet/uv/Udp.h | 1 - .../native/thirdparty/json/include/wpi/json.h | 31 ++----- 14 files changed, 167 insertions(+), 163 deletions(-) delete mode 100644 .github/workflows/documentation.yml create mode 100644 upstream_utils/json_patches/0005-Fix-Doxygen-warnings.patch diff --git a/.github/workflows/documentation.yml b/.github/workflows/documentation.yml deleted file mode 100644 index e9202045d1..0000000000 --- a/.github/workflows/documentation.yml +++ /dev/null @@ -1,67 +0,0 @@ -name: Documentation - -on: [push, workflow_dispatch] - -concurrency: - group: ${{ github.workflow }}-${{ github.ref }} - cancel-in-progress: true - -env: - BASE_PATH: allwpilib/docs - -jobs: - publish: - name: "Documentation - Publish" - runs-on: ubuntu-22.04 - if: github.repository == 'wpilibsuite/allwpilib' && (github.ref == 'refs/heads/main' || (startsWith(github.ref, 'refs/tags/v') && !contains(github.ref, '2027'))) - concurrency: ci-docs-publish - steps: - - uses: actions/checkout@v4 - with: - fetch-depth: 0 - persist-credentials: false - - uses: gradle/actions/wrapper-validation@v4 - - uses: actions/setup-java@v4 - with: - distribution: 'temurin' - java-version: 17 - - name: Set environment variables (Development) - run: | - echo "BRANCH=development" >> $GITHUB_ENV - if: github.ref == 'refs/heads/main' - - name: Set environment variables (Tag) - run: | - echo "EXTRA_GRADLE_ARGS=-PreleaseMode" >> $GITHUB_ENV - echo "BRANCH=beta" >> $GITHUB_ENV - if: startsWith(github.ref, 'refs/tags/v') && !contains(github.ref, '2027') - - name: Set environment variables (Release) - run: | - echo "EXTRA_GRADLE_ARGS=-PreleaseMode" >> $GITHUB_ENV - echo "BRANCH=release" >> $GITHUB_ENV - if: startsWith(github.ref, 'refs/tags/v') && !contains(github.ref, 'alpha') && !contains(github.ref, 'beta') && !contains(github.ref, '2027') - - name: Build with Gradle - run: ./gradlew docs:generateJavaDocs docs:doxygen -PbuildServer ${{ env.EXTRA_GRADLE_ARGS }} - - name: Install SSH Client 🔑 - uses: webfactory/ssh-agent@v0.9.0 - with: - ssh-private-key: ${{ secrets.GH_DEPLOY_KEY }} - - name: Deploy 🚀 - uses: JamesIves/github-pages-deploy-action@v4.6.1 - with: - ssh-key: true - repository-name: wpilibsuite/wpilibsuite.github.io - branch: allwpilib-${{ env.BRANCH }} - clean: true - single-commit: true - folder: docs/build/docs - - name: Trigger Workflow - uses: actions/github-script@v7 - with: - github-token: ${{ secrets.DISPATCH_PAT_TOKEN }} - script: | - github.rest.actions.createWorkflowDispatch({ - owner: context.repo.owner, - repo: 'wpilibsuite.github.io', - workflow_id: 'static.yml', - ref: 'main', - }) diff --git a/.github/workflows/gradle.yml b/.github/workflows/gradle.yml index 5e8b49b54d..145949908a 100644 --- a/.github/workflows/gradle.yml +++ b/.github/workflows/gradle.yml @@ -203,7 +203,7 @@ jobs: run: echo "EXTRA_GRADLE_ARGS=-PreleaseMode" >> $GITHUB_ENV if: startsWith(github.ref, 'refs/tags/v') && !contains(github.ref, '2027') - name: Build with Gradle - run: ./gradlew docs:zipDocs --build-cache -PbuildServer ${{ env.EXTRA_GRADLE_ARGS }} + run: ./gradlew docs:zipDocs --build-cache -PbuildServer -PdocWarningsAsErrors ${{ env.EXTRA_GRADLE_ARGS }} env: ARTIFACTORY_PUBLISH_USERNAME: ${{ secrets.ARTIFACTORY_USERNAME }} ARTIFACTORY_PUBLISH_PASSWORD: ${{ secrets.ARTIFACTORY_PASSWORD }} @@ -212,6 +212,68 @@ jobs: name: Documentation path: docs/build/outputs + publish: + name: "Documentation - Publish" + runs-on: ubuntu-22.04 + if: github.repository == 'wpilibsuite/allwpilib' && (github.ref == 'refs/heads/main' || (startsWith(github.ref, 'refs/tags/v') && !contains(github.ref, '2027'))) + needs: [build-documentation] + concurrency: ci-docs-publish + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + persist-credentials: false + - name: Download docs artifacts + uses: actions/download-artifact@v4 + with: + name: Documentation + - name: Make output directories + run: | + mkdir -p docs/tmp/doxygen/html + mkdir -p docs/tmp/javadoc + - name: Extract docs + run: | + unzip _GROUP_edu_wpi_first_wpilibc_ID_documentation_CLS.zip -d docs/tmp/doxygen/html + unzip _GROUP_edu_wpi_first_wpilibj_ID_documentation_CLS.zip -d docs/tmp/javadoc + - name: Set environment variables (Development) + run: | + echo "BRANCH=development" >> $GITHUB_ENV + if: github.ref == 'refs/heads/main' + - name: Set environment variables (Tag) + run: | + echo "EXTRA_GRADLE_ARGS=-PreleaseMode" >> $GITHUB_ENV + echo "BRANCH=beta" >> $GITHUB_ENV + if: startsWith(github.ref, 'refs/tags/v') && !contains(github.ref, '2027') + - name: Set environment variables (Release) + run: | + echo "EXTRA_GRADLE_ARGS=-PreleaseMode" >> $GITHUB_ENV + echo "BRANCH=release" >> $GITHUB_ENV + if: startsWith(github.ref, 'refs/tags/v') && !contains(github.ref, 'alpha') && !contains(github.ref, 'beta') && !contains(github.ref, '2027') + - name: Install SSH Client 🔑 + uses: webfactory/ssh-agent@v0.9.0 + with: + ssh-private-key: ${{ secrets.GH_DEPLOY_KEY }} + - name: Deploy 🚀 + uses: JamesIves/github-pages-deploy-action@v4.6.1 + with: + ssh-key: true + repository-name: wpilibsuite/wpilibsuite.github.io + branch: allwpilib-${{ env.BRANCH }} + clean: true + single-commit: true + folder: docs/tmp + - name: Trigger Workflow + uses: actions/github-script@v7 + with: + github-token: ${{ secrets.DISPATCH_PAT_TOKEN }} + script: | + github.rest.actions.createWorkflowDispatch({ + owner: context.repo.owner, + repo: 'wpilibsuite.github.io', + workflow_id: 'static.yml', + ref: 'main', + }) + combine: name: Combine needs: [build-docker, build-host, build-documentation] diff --git a/.github/workflows/lint-format.yml b/.github/workflows/lint-format.yml index 938405e552..7075ac11cb 100644 --- a/.github/workflows/lint-format.yml +++ b/.github/workflows/lint-format.yml @@ -123,18 +123,3 @@ jobs: echo '' >> $GITHUB_STEP_SUMMARY echo '```' >> $GITHUB_STEP_SUMMARY if: ${{ failure() }} - - documentation: - name: "Documentation" - runs-on: ubuntu-22.04 - needs: [validation] - steps: - - uses: actions/checkout@v4 - with: - fetch-depth: 0 - - uses: actions/setup-java@v4 - with: - distribution: 'temurin' - java-version: 21 - - name: Build with Gradle - run: ./gradlew docs:zipDocs -PbuildServer -PdocWarningsAsErrors ${{ env.EXTRA_GRADLE_ARGS }} diff --git a/docs/build.gradle b/docs/build.gradle index 93a41ef77f..a500a38961 100644 --- a/docs/build.gradle +++ b/docs/build.gradle @@ -77,66 +77,19 @@ doxygen.sourceSets.main { // apriltag exclude 'apriltag_pose.h' - // Eigen - exclude 'Eigen/**' - exclude 'unsupported/**' - // LLVM - exclude 'wpi/AlignOf.h' - exclude 'wpi/Casting.h' - exclude 'wpi/Chrono.h' exclude 'wpi/Compiler.h' - exclude 'wpi/ConvertUTF.h' - exclude 'wpi/DenseMap.h' - exclude 'wpi/DenseMapInfo.h' - exclude 'wpi/Endian.h' - exclude 'wpi/EpochTracker.h' - exclude 'wpi/Errc.h' - exclude 'wpi/Errno.h' exclude 'wpi/ErrorHandling.h' exclude 'wpi/bit.h' - exclude 'wpi/fs.h' - exclude 'wpi/FunctionExtras.h' - exclude 'wpi/function_ref.h' - exclude 'wpi/Hashing.h' - exclude 'wpi/iterator.h' - exclude 'wpi/iterator_range.h' - exclude 'wpi/ManagedStatic.h' - exclude 'wpi/MapVector.h' - exclude 'wpi/MathExtras.h' - exclude 'wpi/MemAlloc.h' - exclude 'wpi/PointerIntPair.h' - exclude 'wpi/PointerLikeTypeTraits.h' - exclude 'wpi/PointerUnion.h' - exclude 'wpi/raw_os_ostream.h' exclude 'wpi/raw_ostream.h' - exclude 'wpi/SmallPtrSet.h' - exclude 'wpi/SmallSet.h' - exclude 'wpi/SmallString.h' exclude 'wpi/SmallVector.h' exclude 'wpi/StringExtras.h' - exclude 'wpi/StringMap.h' - exclude 'wpi/SwapByteOrder.h' - exclude 'wpi/type_traits.h' - exclude 'wpi/VersionTuple.h' - exclude 'wpi/WindowsError.h' - - // fmtlib - exclude 'fmt/**' // libuv - exclude 'uv.h' exclude 'uv/**' - exclude 'wpinet/uv/**' // json - exclude 'wpi/adl_serializer.h' - exclude 'wpi/byte_container_with_subtype.h' exclude 'wpi/detail/**' - exclude 'wpi/json.h' - exclude 'wpi/json_fwd.h' - exclude 'wpi/ordered_map.h' - exclude 'wpi/thirdparty/**' // mpack exclude 'wpi/mpack.h' diff --git a/upstream_utils/json_patches/0001-Remove-version-from-namespace.patch b/upstream_utils/json_patches/0001-Remove-version-from-namespace.patch index 945e6ea489..9a75050313 100644 --- a/upstream_utils/json_patches/0001-Remove-version-from-namespace.patch +++ b/upstream_utils/json_patches/0001-Remove-version-from-namespace.patch @@ -1,7 +1,7 @@ From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Tyler Veness Date: Fri, 8 Sep 2023 19:21:41 -0700 -Subject: [PATCH 1/4] Remove version from namespace +Subject: [PATCH 1/5] Remove version from namespace --- include/nlohmann/detail/abi_macros.hpp | 45 ++------------------------ diff --git a/upstream_utils/json_patches/0002-Make-serializer-public.patch b/upstream_utils/json_patches/0002-Make-serializer-public.patch index f4d28e0530..f0d49188e3 100644 --- a/upstream_utils/json_patches/0002-Make-serializer-public.patch +++ b/upstream_utils/json_patches/0002-Make-serializer-public.patch @@ -1,7 +1,7 @@ From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Tyler Veness Date: Thu, 7 Sep 2023 22:02:27 -0700 -Subject: [PATCH 2/4] Make serializer public +Subject: [PATCH 2/5] Make serializer public --- include/nlohmann/detail/output/serializer.hpp | 4 +++- diff --git a/upstream_utils/json_patches/0003-Make-dump_escaped-take-std-string_view.patch b/upstream_utils/json_patches/0003-Make-dump_escaped-take-std-string_view.patch index 20a87fd047..6900627940 100644 --- a/upstream_utils/json_patches/0003-Make-dump_escaped-take-std-string_view.patch +++ b/upstream_utils/json_patches/0003-Make-dump_escaped-take-std-string_view.patch @@ -1,7 +1,7 @@ From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Tyler Veness Date: Fri, 8 Sep 2023 21:42:01 -0700 -Subject: [PATCH 3/4] Make dump_escaped() take std::string_view +Subject: [PATCH 3/5] Make dump_escaped() take std::string_view --- include/nlohmann/detail/output/serializer.hpp | 2 +- diff --git a/upstream_utils/json_patches/0004-Add-llvm-stream-support.patch b/upstream_utils/json_patches/0004-Add-llvm-stream-support.patch index 3548041318..a336e8de4d 100644 --- a/upstream_utils/json_patches/0004-Add-llvm-stream-support.patch +++ b/upstream_utils/json_patches/0004-Add-llvm-stream-support.patch @@ -1,7 +1,7 @@ From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: PJ Reiniger Date: Wed, 20 Sep 2023 02:23:10 -0400 -Subject: [PATCH 4/4] Add llvm stream support +Subject: [PATCH 4/5] Add llvm stream support --- .../detail/output/output_adapters.hpp | 26 +++++++++++++++++++ diff --git a/upstream_utils/json_patches/0005-Fix-Doxygen-warnings.patch b/upstream_utils/json_patches/0005-Fix-Doxygen-warnings.patch new file mode 100644 index 0000000000..f1909b2417 --- /dev/null +++ b/upstream_utils/json_patches/0005-Fix-Doxygen-warnings.patch @@ -0,0 +1,93 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Gold856 <117957790+Gold856@users.noreply.github.com> +Date: Tue, 27 May 2025 23:39:02 -0400 +Subject: [PATCH 5/5] Fix Doxygen warnings + +--- + include/nlohmann/json.hpp | 31 ++++++------------------------- + 1 file changed, 6 insertions(+), 25 deletions(-) + +diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp +index a89e2151e589663ba487a462c3d15cd247ff06cf..a5b4f8b4a118c1f5763ec6ba596a8a2d3d5791eb 100644 +--- a/include/nlohmann/json.hpp ++++ b/include/nlohmann/json.hpp +@@ -161,7 +161,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec + using serializer = ::nlohmann::detail::serializer; + + using value_t = detail::value_t; +- /// JSON Pointer, see @ref nlohmann::json_pointer ++ /// JSON Pointer, see @ref json_pointer + using json_pointer = ::nlohmann::json_pointer; + template + using json_serializer = JSONSerializer; +@@ -173,7 +173,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec + using initializer_list_t = std::initializer_list>; + + using input_format_t = detail::input_format_t; +- /// SAX interface type, see @ref nlohmann::json_sax ++ /// SAX interface type, see nlohmann::json_sax + using json_sax_t = json_sax; + + //////////////// +@@ -1606,13 +1606,6 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec + + @throw what @ref json_serializer `from_json()` method throws + +- @liveexample{The example below shows several conversions from JSON values +- to other types. There a few things to note: (1) Floating-point numbers can +- be converted to integers\, (2) A JSON array can be converted to a standard +- `std::vector`\, (3) A JSON object can be converted to C++ +- associative containers such as `std::unordered_map`.,get__ValueType_const} +- + @since version 2.1.0 + */ + template < typename ValueType, +@@ -1678,7 +1671,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec + + @return a copy of *this, converted into @a BasicJsonType + +- @complexity Depending on the implementation of the called `from_json()` ++ Complexity: Depending on the implementation of the called `from_json()` + method. + + @since version 3.2.0 +@@ -1702,7 +1695,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec + + @return a copy of *this + +- @complexity Constant. ++ Complexity: Constant. + + @since version 2.1.0 + */ +@@ -1786,12 +1779,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec + @return pointer to the internally stored JSON value if the requested + pointer type @a PointerType fits to the JSON value; `nullptr` otherwise + +- @complexity Constant. +- +- @liveexample{The example below shows how pointers to internal values of a +- JSON value can be requested. Note that no type conversions are made and a +- `nullptr` is returned if the value and the requested pointer type does not +- match.,get__PointerType} ++ Complexity: Constant. + + @sa see @ref get_ptr() for explicit pointer-member access + +@@ -1883,14 +1871,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec + to the JSON value type (e.g., the JSON value is of type boolean, but a + string is requested); see example below + +- @complexity Linear in the size of the JSON value. +- +- @liveexample{The example below shows several conversions from JSON values +- to other types. There a few things to note: (1) Floating-point numbers can +- be converted to integers\, (2) A JSON array can be converted to a standard +- `std::vector`\, (3) A JSON object can be converted to C++ +- associative containers such as `std::unordered_map`.,operator__ValueType} ++ Complexity: Linear in the size of the JSON value. + + @since version 1.0.0 + */ diff --git a/wpinet/src/main/native/include/wpinet/uv/FsEvent.h b/wpinet/src/main/native/include/wpinet/uv/FsEvent.h index ba8a649595..1eb604da91 100644 --- a/wpinet/src/main/native/include/wpinet/uv/FsEvent.h +++ b/wpinet/src/main/native/include/wpinet/uv/FsEvent.h @@ -49,7 +49,7 @@ class FsEvent final : public HandleImpl { * Start watching the specified path for changes. * * @param path Path to watch for changes - * @param events Bitmask of event flags. Only UV_FS_EVENT_RECURSIVE is + * @param flags Bitmask of event flags. Only UV_FS_EVENT_RECURSIVE is * supported (and only on OSX and Windows). */ void Start(std::string_view path, unsigned int flags = 0); diff --git a/wpinet/src/main/native/include/wpinet/uv/GetNameInfo.h b/wpinet/src/main/native/include/wpinet/uv/GetNameInfo.h index 1bd0f4a594..794c8d7bf2 100644 --- a/wpinet/src/main/native/include/wpinet/uv/GetNameInfo.h +++ b/wpinet/src/main/native/include/wpinet/uv/GetNameInfo.h @@ -87,7 +87,6 @@ void GetNameInfo(Loop& loop, * @param callback Callback function to call when resolution completes * @param addr Initialized `sockaddr_in` or `sockaddr_in6` data structure. * @param flags Optional flags to modify the behavior of `getnameinfo`. - * @return Connection object for the callback */ inline void GetNameInfo(const std::shared_ptr& loop, std::function callback, diff --git a/wpinet/src/main/native/include/wpinet/uv/Stream.h b/wpinet/src/main/native/include/wpinet/uv/Stream.h index 29e58118a6..3455d7a216 100644 --- a/wpinet/src/main/native/include/wpinet/uv/Stream.h +++ b/wpinet/src/main/native/include/wpinet/uv/Stream.h @@ -90,7 +90,6 @@ class Stream : public Handle { * complete. Errors will be reported to the stream error handler. * * @param callback Callback function to call when shutdown completes - * @return Connection object for the callback */ void Shutdown(std::function callback = nullptr); diff --git a/wpinet/src/main/native/include/wpinet/uv/Udp.h b/wpinet/src/main/native/include/wpinet/uv/Udp.h index cfa245f672..f696b06286 100644 --- a/wpinet/src/main/native/include/wpinet/uv/Udp.h +++ b/wpinet/src/main/native/include/wpinet/uv/Udp.h @@ -146,7 +146,6 @@ class Udp final : public HandleImpl { * * @param ip The address to which to bind. * @param port The port to which to bind. - * @param flags Optional additional flags. */ void Connect6(std::string_view ip, unsigned int port); diff --git a/wpiutil/src/main/native/thirdparty/json/include/wpi/json.h b/wpiutil/src/main/native/thirdparty/json/include/wpi/json.h index afeac0ea7e..3f854e6add 100644 --- a/wpiutil/src/main/native/thirdparty/json/include/wpi/json.h +++ b/wpiutil/src/main/native/thirdparty/json/include/wpi/json.h @@ -161,7 +161,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec using serializer = ::wpi::detail::serializer; using value_t = detail::value_t; - /// JSON Pointer, see @ref wpi::json_pointer + /// JSON Pointer, see @ref json_pointer using json_pointer = ::wpi::json_pointer; template using json_serializer = JSONSerializer; @@ -173,7 +173,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec using initializer_list_t = std::initializer_list>; using input_format_t = detail::input_format_t; - /// SAX interface type, see @ref wpi::json_sax + /// SAX interface type, see wpi::json_sax using json_sax_t = json_sax; //////////////// @@ -1606,13 +1606,6 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec @throw what @ref json_serializer `from_json()` method throws - @liveexample{The example below shows several conversions from JSON values - to other types. There a few things to note: (1) Floating-point numbers can - be converted to integers\, (2) A JSON array can be converted to a standard - `std::vector`\, (3) A JSON object can be converted to C++ - associative containers such as `std::unordered_map`.,get__ValueType_const} - @since version 2.1.0 */ template < typename ValueType, @@ -1678,7 +1671,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec @return a copy of *this, converted into @a BasicJsonType - @complexity Depending on the implementation of the called `from_json()` + Complexity: Depending on the implementation of the called `from_json()` method. @since version 3.2.0 @@ -1702,7 +1695,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec @return a copy of *this - @complexity Constant. + Complexity: Constant. @since version 2.1.0 */ @@ -1786,12 +1779,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec @return pointer to the internally stored JSON value if the requested pointer type @a PointerType fits to the JSON value; `nullptr` otherwise - @complexity Constant. - - @liveexample{The example below shows how pointers to internal values of a - JSON value can be requested. Note that no type conversions are made and a - `nullptr` is returned if the value and the requested pointer type does not - match.,get__PointerType} + Complexity: Constant. @sa see @ref get_ptr() for explicit pointer-member access @@ -1883,14 +1871,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec to the JSON value type (e.g., the JSON value is of type boolean, but a string is requested); see example below - @complexity Linear in the size of the JSON value. - - @liveexample{The example below shows several conversions from JSON values - to other types. There a few things to note: (1) Floating-point numbers can - be converted to integers\, (2) A JSON array can be converted to a standard - `std::vector`\, (3) A JSON object can be converted to C++ - associative containers such as `std::unordered_map`.,operator__ValueType} + Complexity: Linear in the size of the JSON value. @since version 1.0.0 */ From 5e7e5306df4eab04307c0d3c2f53775bd86b4e40 Mon Sep 17 00:00:00 2001 From: Joseph Eng <91924258+KangarooKoala@users.noreply.github.com> Date: Tue, 30 Sep 2025 13:57:42 -0700 Subject: [PATCH 05/10] [wpiutil] Update StructSerializable contract (NFC) (#7441) Matches ProtobufSerializable. This is necessary for generic types. --- .../java/edu/wpi/first/util/struct/StructSerializable.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/wpiutil/src/main/java/edu/wpi/first/util/struct/StructSerializable.java b/wpiutil/src/main/java/edu/wpi/first/util/struct/StructSerializable.java index a8d1fdd807..d3f8db5e0a 100644 --- a/wpiutil/src/main/java/edu/wpi/first/util/struct/StructSerializable.java +++ b/wpiutil/src/main/java/edu/wpi/first/util/struct/StructSerializable.java @@ -10,6 +10,7 @@ import edu.wpi.first.util.WPISerializable; * Marker interface to indicate a class is serializable using Struct serialization. * *

While this cannot be enforced by the interface, any class implementing this interface should - * provide a public final static `struct` member variable. + * provide a public final static `struct` member variable, or a static final `getStruct()` method if + * the class is generic. */ public interface StructSerializable extends WPISerializable {} From ca7718cb08e31f55ea62e8f3555f02fdba39aea1 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Thu, 2 Oct 2025 17:35:50 -0700 Subject: [PATCH 06/10] [glass] FMS: Fix reading past end of GSM buffer (#8268) --- glass/src/lib/native/cpp/other/FMS.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/glass/src/lib/native/cpp/other/FMS.cpp b/glass/src/lib/native/cpp/other/FMS.cpp index 86ed0205a7..22de23304b 100644 --- a/glass/src/lib/native/cpp/other/FMS.cpp +++ b/glass/src/lib/native/cpp/other/FMS.cpp @@ -148,11 +148,14 @@ void glass::DisplayFMSReadOnly(FMSModel* model) { ImGui::TextUnformatted("?"); } } - - wpi::SmallString<64> gameSpecificMessageBuf; - std::string_view gameSpecificMessage = - model->GetGameSpecificMessage(gameSpecificMessageBuf); - ImGui::Text("Game Specific: %s", exists ? gameSpecificMessage.data() : "?"); + if (exists) { + wpi::SmallString<64> gsmBuf; + std::string_view gsm = model->GetGameSpecificMessage(gsmBuf); + ImGui::Text("Game Specific: %.*s", static_cast(gsm.size()), + gsm.data()); + } else { + ImGui::TextUnformatted("Game Specific: ?"); + } if (!exists) { ImGui::PopStyleColor(); From 871769c815ed068665e94ad4cd403672855e7d4e Mon Sep 17 00:00:00 2001 From: Joseph Eng <91924258+KangarooKoala@users.noreply.github.com> Date: Thu, 2 Oct 2025 17:36:30 -0700 Subject: [PATCH 07/10] [wpimath] Fix units overload resolution (#8267) --- wpimath/src/main/native/include/units/base.h | 1 + wpimath/src/test/native/cpp/UnitsTest.cpp | 15 +++++++++++++++ 2 files changed, 16 insertions(+) diff --git a/wpimath/src/main/native/include/units/base.h b/wpimath/src/main/native/include/units/base.h index fc5ae4b29d..626d76e16e 100644 --- a/wpimath/src/main/native/include/units/base.h +++ b/wpimath/src/main/native/include/units/base.h @@ -1992,6 +1992,7 @@ namespace units * @param[in] rhs unit to copy. */ template class NlsRhs> + requires traits::is_unit_v && traits::is_unit_v && traits::is_convertible_unit_v constexpr unit_t(const unit_t& rhs) noexcept : nls(units::convert(rhs.m_value), std::true_type() /*store linear value*/) { diff --git a/wpimath/src/test/native/cpp/UnitsTest.cpp b/wpimath/src/test/native/cpp/UnitsTest.cpp index 6357e79103..3b6cd606e3 100644 --- a/wpimath/src/test/native/cpp/UnitsTest.cpp +++ b/wpimath/src/test/native/cpp/UnitsTest.cpp @@ -3513,3 +3513,18 @@ TEST_F(CaseStudies, pythagoreanTheorum) { pow<2>(RightTriangle::b::value()) == pow<2>(RightTriangle::c::value())); } + +TEST(Units, overloadResolution) { + // Slight hack to get nested functions + struct Scope { + static bool f(units::meter_t) { + return true; + }; + + static bool f(units::second_t) { + return false; + }; + }; + // Make sure this properly selects the meter overload + EXPECT_TRUE(Scope::f(1_mm)); +} From 3972b01c5195f98765cb43d55c05822400a77486 Mon Sep 17 00:00:00 2001 From: Sam Carlberg Date: Fri, 3 Oct 2025 20:42:47 -0400 Subject: [PATCH 08/10] Add javac plugin for detecting common error cases at compile time (#8196) Adds a `@NoDiscard` annotation that can be placed on methods to guarantee their return values are used and on types to guarantee that any method returning that type uses the return value. Methods that call `@NoDiscard`-annotated functions can add a `@SuppressWarnings("NoDiscard")` or `@SuppressWarnings("all")` annotation (or annotation on the class declaring that method) to silence the compiler error warnings. --- CMakeLists.txt | 5 + docs/build.gradle | 2 + javacPlugin/BUILD.bazel | 11 + javacPlugin/build.gradle | 17 + .../javacplugin/ReturnValueUsedListener.java | 224 ++++++ .../wpilib/javacplugin/WPILibJavacPlugin.java | 31 + .../services/com.sun.source.util.Plugin | 1 + .../javacplugin/CompileTestOptions.java | 13 + .../ReturnValueUsedListenerTest.java | 638 ++++++++++++++++++ settings.gradle | 2 + wpiannotations/BUILD.bazel | 8 + wpiannotations/CMakeLists.txt | 37 + wpiannotations/build.gradle | 11 + .../java/org/wpilib/annotation/NoDiscard.java | 27 + wpiannotations/wpiannotations-config.cmake | 2 + wpilibNewCommands/BUILD.bazel | 5 + wpilibNewCommands/CMakeLists.txt | 1 + wpilibNewCommands/build.gradle | 2 + .../wpi/first/wpilibj2/command/Command.java | 2 + 19 files changed, 1039 insertions(+) create mode 100644 javacPlugin/BUILD.bazel create mode 100644 javacPlugin/build.gradle create mode 100644 javacPlugin/src/main/java/org/wpilib/javacplugin/ReturnValueUsedListener.java create mode 100644 javacPlugin/src/main/java/org/wpilib/javacplugin/WPILibJavacPlugin.java create mode 100644 javacPlugin/src/main/resources/META-INF/services/com.sun.source.util.Plugin create mode 100644 javacPlugin/src/test/java/org/wpilib/javacplugin/CompileTestOptions.java create mode 100644 javacPlugin/src/test/java/org/wpilib/javacplugin/ReturnValueUsedListenerTest.java create mode 100644 wpiannotations/BUILD.bazel create mode 100644 wpiannotations/CMakeLists.txt create mode 100644 wpiannotations/build.gradle create mode 100644 wpiannotations/src/main/java/org/wpilib/annotation/NoDiscard.java create mode 100644 wpiannotations/wpiannotations-config.cmake diff --git a/CMakeLists.txt b/CMakeLists.txt index 794885abe8..e7592ac89c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -289,6 +289,7 @@ endif() set(FILENAME_DEP_REPLACE "get_filename_component(SELF_DIR \"$\{CMAKE_CURRENT_LIST_FILE\}\" PATH)") set(SELF_DIR "$\{SELF_DIR\}") set(WPIUNITS_DEP_REPLACE_IMPL "find_dependency(wpiunits)") +set(WPIANNOTATIONS_DEP_REPLACE_IMPL "find_dependency(wpiannotations)") set(WPIUTIL_DEP_REPLACE "find_dependency(wpiutil)") add_subdirectory(wpiutil) @@ -308,6 +309,10 @@ if(WITH_WPIMATH) add_subdirectory(wpimath) endif() +if(WITH_JAVA) + add_subdirectory(wpiannotations) +endif() + if(WITH_WPIUNITS AND NOT WITH_WPIMATH) # In case of building wpiunits standalone set(WPIUNITS_DEP_REPLACE ${WPIUNITS_DEP_REPLACE_IMPL}) diff --git a/docs/build.gradle b/docs/build.gradle index a500a38961..423dc96cb2 100644 --- a/docs/build.gradle +++ b/docs/build.gradle @@ -9,6 +9,7 @@ evaluationDependsOn(':cscore') evaluationDependsOn(':epilogue-runtime') evaluationDependsOn(':hal') evaluationDependsOn(':ntcore') +evaluationDependsOn(':wpiannotations') evaluationDependsOn(':wpilibNewCommands') evaluationDependsOn(':wpilibc') evaluationDependsOn(':wpilibj') @@ -174,6 +175,7 @@ task generateJavaDocs(type: Javadoc) { source project(':epilogue-runtime').sourceSets.main.java source project(':hal').sourceSets.main.java source project(':ntcore').sourceSets.main.java + source project(':wpiannotations').sourceSets.main.java source project(':wpilibNewCommands').sourceSets.main.java source project(':wpilibj').sourceSets.main.java source project(':wpimath').sourceSets.main.java diff --git a/javacPlugin/BUILD.bazel b/javacPlugin/BUILD.bazel new file mode 100644 index 0000000000..2c3b786338 --- /dev/null +++ b/javacPlugin/BUILD.bazel @@ -0,0 +1,11 @@ +load("@rules_java//java:defs.bzl", "java_plugin") + +java_plugin( + name = "plugin", + srcs = glob(["src/main/java/**/*.java"]), + resources = glob(["src/main/resources/**"]), + visibility = ["//visibility:public"], + deps = [ + "//wpiannotations", + ], +) diff --git a/javacPlugin/build.gradle b/javacPlugin/build.gradle new file mode 100644 index 0000000000..89adb15320 --- /dev/null +++ b/javacPlugin/build.gradle @@ -0,0 +1,17 @@ +ext { + useJava = true + useCpp = false + baseId = 'wpilibj-javac-plugin' + groupId = 'org.wpilib' + + nativeName = '' + devMain = '' +} + +apply from: "${rootDir}/shared/java/javacommon.gradle" + +dependencies { + implementation project(':wpiannotations') + testImplementation 'com.google.testing.compile:compile-testing:+' + testImplementation project(':wpilibNewCommands') +} diff --git a/javacPlugin/src/main/java/org/wpilib/javacplugin/ReturnValueUsedListener.java b/javacPlugin/src/main/java/org/wpilib/javacplugin/ReturnValueUsedListener.java new file mode 100644 index 0000000000..4481e933fa --- /dev/null +++ b/javacPlugin/src/main/java/org/wpilib/javacplugin/ReturnValueUsedListener.java @@ -0,0 +1,224 @@ +// Copyright (c) FIRST and other WPILib contributors. +// Open Source Software; you can modify and/or share it under the terms of +// the WPILib BSD license file in the root directory of this project. + +package org.wpilib.javacplugin; + +import com.sun.source.tree.CompilationUnitTree; +import com.sun.source.tree.MethodInvocationTree; +import com.sun.source.tree.NewClassTree; +import com.sun.source.tree.Tree; +import com.sun.source.util.JavacTask; +import com.sun.source.util.TaskEvent; +import com.sun.source.util.TaskListener; +import com.sun.source.util.TreeScanner; +import com.sun.source.util.Trees; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import javax.lang.model.element.ElementKind; +import javax.lang.model.element.ExecutableElement; +import javax.lang.model.element.TypeElement; +import javax.lang.model.type.DeclaredType; +import javax.lang.model.type.TypeKind; +import javax.lang.model.type.TypeMirror; +import javax.tools.Diagnostic; +import org.wpilib.annotation.NoDiscard; + +/** Checks for usages of methods that require their return values to be used. */ +public class ReturnValueUsedListener implements TaskListener { + private final JavacTask m_task; + private final Set m_visitedCUs = new HashSet<>(); + + public ReturnValueUsedListener(JavacTask task) { + m_task = task; + } + + @Override + public void finished(TaskEvent e) { + // We override `finished` instead of `started` because we want to run after the + // ANALYZE attribution phase has completed and assigned types to elements in the AST + // Track the visited CUs to avoid re-processing the same CU multiple times when we call + // `Trees.getElement()` on a tree path. + if (e.getKind() == TaskEvent.Kind.ANALYZE && m_visitedCUs.add(e.getCompilationUnit())) { + e.getCompilationUnit().accept(new Scanner(e.getCompilationUnit()), null); + } + } + + private final class Scanner extends TreeScanner { + private final CompilationUnitTree m_root; + private final Trees m_trees; + + Scanner(CompilationUnitTree compilationUnit) { + m_root = compilationUnit; + m_trees = Trees.instance(m_task); + } + + @Override + public Void visitMethodInvocation(MethodInvocationTree node, Void unused) { + checkIgnoredExpression(node); + return super.visitMethodInvocation(node, unused); + } + + @Override + public Void visitNewClass(NewClassTree node, Void unused) { + checkIgnoredExpression(node); + return super.visitNewClass(node, unused); + } + + /** + * Common logic for both method invocations and constructor calls when they appear as + * stand-alone expression statements (i.e., their result is ignored). + */ + private void checkIgnoredExpression(Tree node) { + var path = m_trees.getPath(m_root, node); + + // Walk the tree upwards to see if the node is directly or indirectly annotated with + // @SuppressWarnings("NoDiscard") or @SuppressWarnings("all"). If so, then we ignore any + // @NoDiscard messages for this node + for (var currentPath = path; currentPath != null; currentPath = currentPath.getParentPath()) { + var element = m_trees.getElement(currentPath); + if (element == null) { + continue; + } + + if (element.getAnnotation(SuppressWarnings.class) != null) { + String[] suppressions = element.getAnnotation(SuppressWarnings.class).value(); + for (String suppression : suppressions) { + if ("NoDiscard".equals(suppression) || "all".equals(suppression)) { + return; + } + } + } + } + + var parentPath = (path == null) ? null : path.getParentPath(); + if (parentPath == null || parentPath.getLeaf().getKind() != Tree.Kind.EXPRESSION_STATEMENT) { + // If the parent node is an expression statement, then the value is ignored. + // Otherwise, the value is used and we can ignore this site. + return; + } + + // Resolve the static type of the expression + TypeMirror type = getType(node); + if (type == null || type.getKind() == TypeKind.VOID) { + // Skip void (e.g., void-returning methods) + return; + } + + // Check @NoDiscard on the invoked executable (method or constructor) + var invoked = getInvokedExecutable(node); + if (invoked != null) { + List messages = getNoDiscardMessages(invoked); + for (String msg : messages) { + m_trees.printMessage(Diagnostic.Kind.ERROR, msg, node, m_root); + } + } + } + + private TypeMirror getType(Tree node) { + var path = m_trees.getPath(m_root, node); + if (path == null) { + return null; + } + // Requires running after ANALYZE attribution has completed for this CU. + return m_trees.getTypeMirror(path); + } + + private ExecutableElement getInvokedExecutable(Tree node) { + var path = m_trees.getPath(m_root, node); + if (path == null) { + return null; + } + var el = m_trees.getElement(path); + return (el instanceof ExecutableElement ee) ? ee : null; + } + + /** + * Collects all @NoDiscard messages applicable to the given executable: - The method/constructor + * itself (if annotated) - The return type (if declared) including its superclasses and all + * implemented interfaces Returns formatted diagnostics messages ready to print. + */ + private List getNoDiscardMessages(ExecutableElement method) { + List messages = new ArrayList<>(); + + // 1) Method-level @NoDiscard + var methodNoDiscard = method.getAnnotation(NoDiscard.class); + if (methodNoDiscard != null) { + String msg = methodNoDiscard.value(); + if (msg.isEmpty()) { + messages.add("Result of @NoDiscard method is ignored"); + } else { + messages.add(msg); + } + } + + // 2) Type-level @NoDiscard (classes + interfaces recursively) + TypeElement targetType = null; + if (method.getKind() == ElementKind.CONSTRUCTOR) { + // For constructors, the "return type" is the enclosing type + var enclosing = method.getEnclosingElement(); + if (enclosing instanceof TypeElement te) { + targetType = te; + } + } else { + var returnType = method.getReturnType(); + if (returnType instanceof DeclaredType dt && dt.asElement() instanceof TypeElement te) { + targetType = te; + } + } + if (targetType != null) { + Set seen = new HashSet<>(); + collectNoDiscardMessagesFromTypeHierarchy(targetType, seen, messages); + } + + return messages; + } + + /** + * Searches for @NoDiscard on the provided type element, its superclasses, and all implemented + * interfaces (recursively). Appends formatted messages to the provided list for every match. + * + * @param type The type element to search + * @param seen A set of type elements that have already been searched + * @param out The list to append messages to + */ + private void collectNoDiscardMessagesFromTypeHierarchy( + TypeElement type, Set seen, List out) { + if (type == null || !seen.add(type)) { + return; + } + + // Check this type directly + var typeNoDiscard = type.getAnnotation(NoDiscard.class); + if (typeNoDiscard != null) { + String message = typeNoDiscard.value(); + if (message.isEmpty()) { + out.add( + "Result of method returning @NoDiscard type %s is ignored" + .formatted(type.getQualifiedName())); + } else { + out.add(message); + } + } + + // Check superclass chain + var superMirror = type.getSuperclass(); + if (superMirror != null && superMirror.getKind() != TypeKind.NONE) { + var superEl = m_task.getTypes().asElement(superMirror); + if (superEl instanceof TypeElement ste) { + collectNoDiscardMessagesFromTypeHierarchy(ste, seen, out); + } + } + + // Check all implemented interfaces (recursively) + for (var iface : type.getInterfaces()) { + var ifaceEl = m_task.getTypes().asElement(iface); + if (ifaceEl instanceof TypeElement ite) { + collectNoDiscardMessagesFromTypeHierarchy(ite, seen, out); + } + } + } + } +} diff --git a/javacPlugin/src/main/java/org/wpilib/javacplugin/WPILibJavacPlugin.java b/javacPlugin/src/main/java/org/wpilib/javacplugin/WPILibJavacPlugin.java new file mode 100644 index 0000000000..87bc8f43f4 --- /dev/null +++ b/javacPlugin/src/main/java/org/wpilib/javacplugin/WPILibJavacPlugin.java @@ -0,0 +1,31 @@ +// Copyright (c) FIRST and other WPILib contributors. +// Open Source Software; you can modify and/or share it under the terms of +// the WPILib BSD license file in the root directory of this project. + +package org.wpilib.javacplugin; + +import com.sun.source.util.JavacTask; +import com.sun.source.util.Plugin; + +/** + * A javac compiler plugin that adds compiler warnings for incorrect usage of WPILib types. Also + * supports WPILib's custom annotations like @NoDiscard. + */ +public class WPILibJavacPlugin implements Plugin { + @Override + public String getName() { + return "WPILib"; + } + + @Override + public void init(JavacTask task, String... args) { + task.addTaskListener(new ReturnValueUsedListener(task)); + } + + @Override + public boolean autoStart() { + // autoStart means we don't need to manually pass -Xplugin:WPILib to the javac compiler args + // for the plugin to run + return true; + } +} diff --git a/javacPlugin/src/main/resources/META-INF/services/com.sun.source.util.Plugin b/javacPlugin/src/main/resources/META-INF/services/com.sun.source.util.Plugin new file mode 100644 index 0000000000..e1b3dbab39 --- /dev/null +++ b/javacPlugin/src/main/resources/META-INF/services/com.sun.source.util.Plugin @@ -0,0 +1 @@ +org.wpilib.javacplugin.WPILibJavacPlugin diff --git a/javacPlugin/src/test/java/org/wpilib/javacplugin/CompileTestOptions.java b/javacPlugin/src/test/java/org/wpilib/javacplugin/CompileTestOptions.java new file mode 100644 index 0000000000..a5356b4c81 --- /dev/null +++ b/javacPlugin/src/test/java/org/wpilib/javacplugin/CompileTestOptions.java @@ -0,0 +1,13 @@ +// Copyright (c) FIRST and other WPILib contributors. +// Open Source Software; you can modify and/or share it under the terms of +// the WPILib BSD license file in the root directory of this project. + +package org.wpilib.javacplugin; + +import java.util.List; + +public class CompileTestOptions { + public static final int kJavaVersion = 17; + public static final List kJavaVersionOptions = + List.of("-source", kJavaVersion, "-target", kJavaVersion); +} diff --git a/javacPlugin/src/test/java/org/wpilib/javacplugin/ReturnValueUsedListenerTest.java b/javacPlugin/src/test/java/org/wpilib/javacplugin/ReturnValueUsedListenerTest.java new file mode 100644 index 0000000000..65a21ce2f0 --- /dev/null +++ b/javacPlugin/src/test/java/org/wpilib/javacplugin/ReturnValueUsedListenerTest.java @@ -0,0 +1,638 @@ +// Copyright (c) FIRST and other WPILib contributors. +// Open Source Software; you can modify and/or share it under the terms of +// the WPILib BSD license file in the root directory of this project. + +package org.wpilib.javacplugin; + +import static com.google.testing.compile.CompilationSubject.assertThat; +import static com.google.testing.compile.Compiler.javac; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.wpilib.javacplugin.CompileTestOptions.kJavaVersionOptions; + +import com.google.testing.compile.Compilation; +import com.google.testing.compile.JavaFileObjects; +import org.junit.jupiter.api.Test; + +class ReturnValueUsedListenerTest { + @Test + void nodiscardReturnValueIsUsed() { + String source = + """ + package frc.robot; + + import org.wpilib.annotation.NoDiscard; + + class Example { + @NoDiscard + int getI() { return 0; } + + void usage() { + int i = getI(); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).succeededWithoutWarnings(); + } + + @Test + void nodiscardReturnValueUnused() { + String source = + """ + package frc.robot; + + import org.wpilib.annotation.NoDiscard; + + class Example { + @NoDiscard + int getI() { return 0; } + + void usage() { + getI(); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).failed(); + assertEquals(1, compilation.errors().size()); + var error = compilation.errors().get(0); + assertEquals("Result of @NoDiscard method is ignored", error.getMessage(null)); + } + + @Test + void nodiscardOnClass() { + String source = + """ + package frc.robot; + + import org.wpilib.annotation.NoDiscard; + + @NoDiscard + class Example { + Example getExample() { return new Example(); } + + void usage() { + getExample(); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).failed(); + assertEquals(1, compilation.errors().size()); + var error = compilation.errors().get(0); + assertEquals( + "Result of method returning @NoDiscard type frc.robot.Example is ignored", + error.getMessage(null)); + } + + @Test + void nodiscardOnClassCustomMessage() { + String source = + """ + package frc.robot; + + import org.wpilib.annotation.NoDiscard; + + @NoDiscard("Custom message") + class Example { + Example getExample() { return new Example(); } + + void usage() { + getExample(); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).failed(); + assertEquals(1, compilation.errors().size()); + var error = compilation.errors().get(0); + assertEquals("Custom message", error.getMessage(null)); + } + + @Test + void nodiscardOnClassAndMethod() { + String source = + """ + package frc.robot; + + import org.wpilib.annotation.NoDiscard; + + @NoDiscard + class Example { + @NoDiscard + Example getExample() { return new Example(); } + + void usage() { + getExample(); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).failed(); + assertEquals(2, compilation.errors().size()); + var error1 = compilation.errors().get(0); + var error2 = compilation.errors().get(1); + assertEquals("Result of @NoDiscard method is ignored", error1.getMessage(null)); + assertEquals( + "Result of method returning @NoDiscard type frc.robot.Example is ignored", + error2.getMessage(null)); + } + + @Test + void nodiscardOnInheritedClass() { + String source = + """ + package frc.robot; + + import org.wpilib.annotation.NoDiscard; + + @NoDiscard("Objects of type `Base` must be used") + abstract class Base { } + + class Example extends Base { + Example getExample() { return new Example(); } + + void usage() { + getExample(); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).failed(); + assertEquals(1, compilation.errors().size()); + var error = compilation.errors().get(0); + assertEquals("Objects of type `Base` must be used", error.getMessage(null)); + } + + @Test + void nodiscardOnSingleInterface() { + String source = + """ + package frc.robot; + + import org.wpilib.annotation.NoDiscard; + + @NoDiscard("Objects implementing `I` must be used") + interface I { } + + class Example implements I { + Example getExample() { return new Example(); } + + void usage() { + getExample(); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).failed(); + assertEquals(1, compilation.errors().size()); + var error = compilation.errors().get(0); + assertEquals("Objects implementing `I` must be used", error.getMessage(null)); + } + + @Test + void nodiscardOnMultipleInterfaces() { + String source = + """ + package frc.robot; + + import org.wpilib.annotation.NoDiscard; + + @NoDiscard("Objects implementing `I` must be used") + interface I { } + + @NoDiscard("Objects implementing `I2` must be used") + interface I2 { } + + class Example implements I, I2 { + Example getExample() { return new Example(); } + + void usage() { + getExample(); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).failed(); + assertEquals(2, compilation.errors().size()); + var error1 = compilation.errors().get(0); + var error2 = compilation.errors().get(1); + assertEquals("Objects implementing `I` must be used", error1.getMessage(null)); + assertEquals("Objects implementing `I2` must be used", error2.getMessage(null)); + } + + @Test + void nodiscardCustomMessage() { + String source = + """ + package frc.robot; + + import org.wpilib.annotation.NoDiscard; + + class Example { + @NoDiscard("Custom message") + int getI() { return 0; } + + void usage() { + getI(); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).failed(); + assertEquals(1, compilation.errors().size()); + var error = compilation.errors().get(0); + assertEquals("Custom message", error.getMessage(null)); + } + + @Test + void nodiscardMessageEmptyString() { + String source = + """ + package frc.robot; + + import org.wpilib.annotation.NoDiscard; + + class Example { + @NoDiscard("") + int getI() { return 0; } + + void usage() { + getI(); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).failed(); + assertEquals(1, compilation.errors().size()); + var error = compilation.errors().get(0); + assertEquals("Result of @NoDiscard method is ignored", error.getMessage(null)); + } + + @Test + void nodiscardOnVoidMethod() { + String source = + """ + package frc.robot; + + import org.wpilib.annotation.NoDiscard; + + class Example { + @NoDiscard + void voidMethod() { } + + void usage() { + voidMethod(); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).succeededWithoutWarnings(); + } + + @Test + void suppressWarningsOnNoDiscardMethod() { + String source = + """ + package frc.robot; + + import org.wpilib.annotation.NoDiscard; + + class Example { + @NoDiscard + Object get() { return null; } + + @SuppressWarnings("NoDiscard") + void usage() { + get(); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).succeededWithoutWarnings(); + } + + @Test + void suppressWarningsAllOnNoDiscardMethod() { + String source = + """ + package frc.robot; + + import org.wpilib.annotation.NoDiscard; + + class Example { + @NoDiscard + Object get() { return null; } + + @SuppressWarnings("all") + void usage() { + get(); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).succeededWithoutWarnings(); + } + + @Test + void suppressWarningsOnNoDiscardClass() { + String source = + """ + package frc.robot; + + import org.wpilib.annotation.NoDiscard; + + @SuppressWarnings("NoDiscard") + class Example { + @NoDiscard + Object get() { return null; } + + void usage() { + get(); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).succeededWithoutWarnings(); + } + + @Test + void suppressWarningsAllOnNoDiscardClass() { + String source = + """ + package frc.robot; + + import org.wpilib.annotation.NoDiscard; + + @SuppressWarnings("all") + class Example { + @NoDiscard + Object get() { return null; } + + void usage() { + get(); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).succeededWithoutWarnings(); + } + + @Test + void commandsv2CommandFactoryResultIsAssigned() { + String source = + """ + package frc.robot; + + import edu.wpi.first.wpilibj2.command.Command; + import edu.wpi.first.wpilibj2.command.Commands; + import org.wpilib.annotation.NoDiscard; + + class Example { + Command getCommand() { + return Commands.print(""); + } + + void usage() { + Command theCommand = getCommand(); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).succeededWithoutWarnings(); + } + + @Test + void commandsv2CommandFactoryResultIsPassed() { + String source = + """ + package frc.robot; + + import edu.wpi.first.wpilibj2.command.Command; + import edu.wpi.first.wpilibj2.command.Commands; + import org.wpilib.annotation.NoDiscard; + + class Example { + Command getCommand() { + return Commands.print(""); + } + + void usage() { + System.out.println(getCommand()); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).succeededWithoutWarnings(); + } + + @Test + void commandsv2CommandFactoryResultIsChainedAndUsed() { + String source = + """ + package frc.robot; + + import edu.wpi.first.wpilibj2.command.Command; + import edu.wpi.first.wpilibj2.command.Commands; + import org.wpilib.annotation.NoDiscard; + + class Example { + Command getCommand() { + return Commands.print(""); + } + + void usage() { + Command theCommand = getCommand().withName("The name"); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).succeededWithoutWarnings(); + } + + @Test + void commandsv2CommandFactoryResultNotUsed() { + String source = + """ + package frc.robot; + + import edu.wpi.first.wpilibj2.command.Command; + import edu.wpi.first.wpilibj2.command.Commands; + import org.wpilib.annotation.NoDiscard; + + class Example { + Command getCommand() { + return Commands.print(""); + } + + void usage() { + getCommand(); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).failed(); + assertEquals(1, compilation.errors().size()); + var error = compilation.errors().get(0); + assertEquals( + "Commands must be used! Did you mean to bind it to a trigger?", error.getMessage(null)); + } + + @Test + void commandsv2CommandFactoryResultIsChainedAndNotUsed() { + String source = + """ + package frc.robot; + + import edu.wpi.first.wpilibj2.command.Command; + import edu.wpi.first.wpilibj2.command.Commands; + import org.wpilib.annotation.NoDiscard; + + class Example { + Command getCommand() { + return Commands.print(""); + } + + void usage() { + getCommand().withName("The name"); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).failed(); + assertEquals(1, compilation.errors().size()); + var error = compilation.errors().get(0); + assertEquals( + "Commands must be used! Did you mean to bind it to a trigger?", error.getMessage(null)); + } + + @Test + void commandsv2NewCommandInstanceNotUsed() { + String source = + """ + package frc.robot; + + import edu.wpi.first.wpilibj2.command.Command; + import edu.wpi.first.wpilibj2.command.Commands; + import edu.wpi.first.wpilibj2.command.WaitCommand; + import org.wpilib.annotation.NoDiscard; + + class Example { + void usage() { + new WaitCommand(1); + } + } + """; + + Compilation compilation = + javac() + .withOptions(kJavaVersionOptions) + .compile(JavaFileObjects.forSourceString("frc.robot.Example", source)); + + assertThat(compilation).failed(); + assertEquals(1, compilation.errors().size()); + var error = compilation.errors().get(0); + assertEquals( + "Commands must be used! Did you mean to bind it to a trigger?", error.getMessage(null)); + } +} diff --git a/settings.gradle b/settings.gradle index 2064a767cd..3c34412c98 100644 --- a/settings.gradle +++ b/settings.gradle @@ -33,6 +33,8 @@ include 'wpilibcIntegrationTests' include 'wpilibjExamples' include 'wpilibjIntegrationTests' include 'wpilibj' +include 'javacPlugin' +include 'wpiannotations' include 'wpiunits' include 'crossConnIntegrationTests' include 'fieldImages' diff --git a/wpiannotations/BUILD.bazel b/wpiannotations/BUILD.bazel new file mode 100644 index 0000000000..ee4b2401c0 --- /dev/null +++ b/wpiannotations/BUILD.bazel @@ -0,0 +1,8 @@ +load("@rules_java//java:defs.bzl", "java_library") + +java_library( + name = "wpiannotations", + srcs = glob(["src/main/java/**/*.java"]), + visibility = ["//visibility:public"], + deps = [], +) diff --git a/wpiannotations/CMakeLists.txt b/wpiannotations/CMakeLists.txt new file mode 100644 index 0000000000..11eabdbc29 --- /dev/null +++ b/wpiannotations/CMakeLists.txt @@ -0,0 +1,37 @@ +project(wpiannotations) + +# Java bindings +if(WITH_JAVA) + include(UseJava) + + file(GLOB_RECURSE JAVA_SOURCES src/main/java/*.java) + + add_jar( + wpiannotations_jar + ${JAVA_SOURCES} + OUTPUT_NAME wpiannotations + OUTPUT_DIR ${WPILIB_BINARY_DIR}/${java_lib_dest} + ) + set_property(TARGET wpiannotations_jar PROPERTY FOLDER "java") + + install_jar(wpiannotations_jar DESTINATION ${java_lib_dest}) + install_jar_exports( + TARGETS wpiannotations_jar + FILE wpiannotations.cmake + DESTINATION share/wpiannotations + ) + install(FILES wpiannotations-config.cmake DESTINATION share/wpiannotations) +endif() + +if(WITH_JAVA_SOURCE) + include(UseJava) + include(CreateSourceJar) + add_source_jar( + wpiannotations_src_jar + BASE_DIRECTORIES ${CMAKE_CURRENT_SOURCE_DIR}/src/main/java + OUTPUT_NAME wpiannotations-sources + ) + set_property(TARGET wpiannotations_src_jar PROPERTY FOLDER "java") + + install_jar(wpiannotations_src_jar DESTINATION ${java_lib_dest}) +endif() diff --git a/wpiannotations/build.gradle b/wpiannotations/build.gradle new file mode 100644 index 0000000000..2a7a0881d7 --- /dev/null +++ b/wpiannotations/build.gradle @@ -0,0 +1,11 @@ +ext { + useJava = true + useCpp = false + baseId = 'annotations' + groupId = 'org.wpilib' + + nativeName = '' + devMain = '' +} + +apply from: "${rootDir}/shared/java/javacommon.gradle" diff --git a/wpiannotations/src/main/java/org/wpilib/annotation/NoDiscard.java b/wpiannotations/src/main/java/org/wpilib/annotation/NoDiscard.java new file mode 100644 index 0000000000..408d1fb861 --- /dev/null +++ b/wpiannotations/src/main/java/org/wpilib/annotation/NoDiscard.java @@ -0,0 +1,27 @@ +// Copyright (c) FIRST and other WPILib contributors. +// Open Source Software; you can modify and/or share it under the terms of +// the WPILib BSD license file in the root directory of this project. + +package org.wpilib.annotation; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Marks a method as returning a value that must be used. The WPILib compiler plugin will check for + * uses of methods with this annotation and report a compiler error if the value is unused. Marking + * a class or interface as {@code @NoDiscard} will act as if any method that returns that type or + * any subclass or implementor of that type has been marked with {@code @NoDiscard}. + */ +@Target({ElementType.METHOD, ElementType.TYPE}) +@Retention(RetentionPolicy.CLASS) // needs to be stored in the class for use by libraries +public @interface NoDiscard { + /** + * An error message to display if the return value is not used. + * + * @return The error message. + */ + String value() default ""; +} diff --git a/wpiannotations/wpiannotations-config.cmake b/wpiannotations/wpiannotations-config.cmake new file mode 100644 index 0000000000..fa28e4a9c2 --- /dev/null +++ b/wpiannotations/wpiannotations-config.cmake @@ -0,0 +1,2 @@ +get_filename_component(SELF_DIR "${CMAKE_CURRENT_LIST_FILE}" PATH) +include(${SELF_DIR}/wpiannotations.cmake) diff --git a/wpilibNewCommands/BUILD.bazel b/wpilibNewCommands/BUILD.bazel index 6add11137a..fc9326baf2 100644 --- a/wpilibNewCommands/BUILD.bazel +++ b/wpilibNewCommands/BUILD.bazel @@ -37,11 +37,14 @@ cc_library( java_library( name = "wpilibNewCommands-java", srcs = glob(["src/main/java/**/*.java"]) + [":generated_java"], + exported_plugins = ["//javacPlugin:plugin"], + plugins = ["//javacPlugin:plugin"], visibility = ["//visibility:public"], deps = [ "//cscore:cscore-java", "//hal:hal-java", "//ntcore:networktables-java", + "//wpiannotations", "//wpilibj", "//wpimath:wpimath-java", "//wpinet:wpinet-java", @@ -79,8 +82,10 @@ java_binary( srcs = ["src/dev/java/edu/wpi/first/wpilibj2/commands/DevMain.java"], main_class = "edu.wpi.first.wpilibj2.commands.DevMain", deps = [ + ":wpilibNewCommands-java", "//hal:hal-java", "//ntcore:networktables-java", + "//wpiannotations", "//wpimath:wpimath-java", "//wpiutil:wpiutil-java", ], diff --git a/wpilibNewCommands/CMakeLists.txt b/wpilibNewCommands/CMakeLists.txt index f7c664802e..f7959bff80 100644 --- a/wpilibNewCommands/CMakeLists.txt +++ b/wpilibNewCommands/CMakeLists.txt @@ -22,6 +22,7 @@ if(WITH_JAVA) wpiunits_jar wpiutil_jar wpilibj_jar + wpiannotations_jar OUTPUT_NAME wpilibNewCommands OUTPUT_DIR ${WPILIB_BINARY_DIR}/${java_lib_dest} ) diff --git a/wpilibNewCommands/build.gradle b/wpilibNewCommands/build.gradle index 884c18b65c..fb3cf2dbb3 100644 --- a/wpilibNewCommands/build.gradle +++ b/wpilibNewCommands/build.gradle @@ -21,7 +21,9 @@ dependencies { implementation project(':hal') implementation project(':wpimath') implementation project(':wpilibj') + implementation project(':wpiannotations') testImplementation 'org.mockito:mockito-core:4.1.0' + annotationProcessor project(':javacPlugin') } sourceSets.main.java.srcDir "${projectDir}/src/generated/main/java" diff --git a/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/Command.java b/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/Command.java index d46ce313c6..68944440e0 100644 --- a/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/Command.java +++ b/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/Command.java @@ -16,6 +16,7 @@ import java.util.Collection; import java.util.HashSet; import java.util.Set; import java.util.function.BooleanSupplier; +import org.wpilib.annotation.NoDiscard; /** * A state machine representing a complete action to be performed by the robot. Commands are run by @@ -27,6 +28,7 @@ import java.util.function.BooleanSupplier; * *

This class is provided by the NewCommands VendorDep */ +@NoDiscard("Commands must be used! Did you mean to bind it to a trigger?") public abstract class Command implements Sendable { /** Requirements set. */ private final Set m_requirements = new HashSet<>(); From f1b9be551be8909a94f2d39a1a0c79b467a1965f Mon Sep 17 00:00:00 2001 From: Peter Lilley Date: Sat, 4 Oct 2025 02:13:55 -0400 Subject: [PATCH 09/10] [wpiutil] Add reverse/bidirectional iterators to wpi::circular_buffer (#8275) Use std::reverse_iterator<> to create reverse iterators, make other iterators bidirectional to allow for this. Added unit tests. --- .../main/native/include/wpi/circular_buffer.h | 84 ++++++++++++++++++- .../include/wpi/static_circular_buffer.h | 71 ++++++++++++++-- .../test/native/cpp/CircularBufferTest.cpp | 14 ++++ .../native/cpp/StaticCircularBufferTest.cpp | 14 ++++ 4 files changed, 175 insertions(+), 8 deletions(-) diff --git a/wpiutil/src/main/native/include/wpi/circular_buffer.h b/wpiutil/src/main/native/include/wpi/circular_buffer.h index a8eb627d69..4fee596755 100644 --- a/wpiutil/src/main/native/include/wpi/circular_buffer.h +++ b/wpiutil/src/main/native/include/wpi/circular_buffer.h @@ -33,7 +33,7 @@ class circular_buffer { class iterator { public: - using iterator_category = std::forward_iterator_tag; + using iterator_category = std::bidirectional_iterator_tag; using value_type = T; using difference_type = std::ptrdiff_t; using pointer = T*; @@ -51,6 +51,15 @@ class circular_buffer { ++(*this); return retval; } + constexpr iterator& operator--() { + --m_index; + return *this; + } + constexpr iterator operator--(int) { + iterator retval = *this; + --(*this); + return retval; + } constexpr bool operator==(const iterator&) const = default; constexpr reference operator*() { return (*m_buffer)[m_index]; } @@ -61,7 +70,7 @@ class circular_buffer { class const_iterator { public: - using iterator_category = std::forward_iterator_tag; + using iterator_category = std::bidirectional_iterator_tag; using value_type = T; using difference_type = std::ptrdiff_t; using pointer = T*; @@ -79,6 +88,15 @@ class circular_buffer { ++(*this); return retval; } + constexpr const_iterator& operator--() { + --m_index; + return *this; + } + constexpr const_iterator operator--(int) { + const_iterator retval = *this; + --(*this); + return retval; + } constexpr bool operator==(const const_iterator&) const = default; constexpr const_reference operator*() const { return (*m_buffer)[m_index]; } @@ -87,21 +105,83 @@ class circular_buffer { size_t m_index; }; + using reverse_iterator = std::reverse_iterator; + using const_reverse_iterator = std::reverse_iterator; + + /** + * Returns begin iterator. + */ constexpr iterator begin() { return iterator(this, 0); } + + /** + * Returns end iterator. + */ constexpr iterator end() { return iterator(this, ::wpi::circular_buffer::size()); } + /** + * Returns const begin iterator. + */ constexpr const_iterator begin() const { return const_iterator(this, 0); } + + /** + * Returns const end iterator. + */ constexpr const_iterator end() const { return const_iterator(this, ::wpi::circular_buffer::size()); } + /** + * Returns const begin iterator. + */ constexpr const_iterator cbegin() const { return const_iterator(this, 0); } + + /** + * Returns const end iterator. + */ constexpr const_iterator cend() const { return const_iterator(this, ::wpi::circular_buffer::size()); } + /** + * Returns reverse begin iterator. + */ + constexpr reverse_iterator rbegin() { return reverse_iterator(end()); } + + /** + * Returns reverse end iterator. + */ + constexpr reverse_iterator rend() { return reverse_iterator(begin()); } + + /** + * Returns const reverse begin iterator. + */ + constexpr const_reverse_iterator rbegin() const { + return const_reverse_iterator(end()); + } + + /** + * Returns const reverse end iterator. + */ + constexpr const_reverse_iterator rend() const { + return const_reverse_iterator(begin()); + } + + /** + * Returns const reverse begin iterator. + */ + constexpr const_reverse_iterator crbegin() const { + return const_reverse_iterator(cend()); + } + + /** + * Returns const reverse end iterator. + */ + constexpr const_reverse_iterator crend() const { + return const_reverse_iterator(cbegin()); + } + /** * Returns number of elements in buffer */ diff --git a/wpiutil/src/main/native/include/wpi/static_circular_buffer.h b/wpiutil/src/main/native/include/wpi/static_circular_buffer.h index 73817c6dbc..133c9e3a67 100644 --- a/wpiutil/src/main/native/include/wpi/static_circular_buffer.h +++ b/wpiutil/src/main/native/include/wpi/static_circular_buffer.h @@ -24,7 +24,7 @@ class static_circular_buffer { class iterator { public: - using iterator_category = std::forward_iterator_tag; + using iterator_category = std::bidirectional_iterator_tag; using value_type = T; using difference_type = std::ptrdiff_t; using pointer = T*; @@ -42,6 +42,15 @@ class static_circular_buffer { ++(*this); return retval; } + constexpr iterator& operator--() { + --m_index; + return *this; + } + constexpr iterator operator--(int) { + iterator retval = *this; + --(*this); + return retval; + } constexpr bool operator==(const iterator&) const = default; constexpr reference operator*() { return (*m_buffer)[m_index]; } @@ -52,7 +61,7 @@ class static_circular_buffer { class const_iterator { public: - using iterator_category = std::forward_iterator_tag; + using iterator_category = std::bidirectional_iterator_tag; using value_type = T; using difference_type = std::ptrdiff_t; using pointer = T*; @@ -70,6 +79,15 @@ class static_circular_buffer { ++(*this); return retval; } + constexpr const_iterator& operator--() { + --m_index; + return *this; + } + constexpr const_iterator operator--(int) { + const_iterator retval = *this; + --(*this); + return retval; + } constexpr bool operator==(const const_iterator&) const = default; constexpr const_reference operator*() const { return (*m_buffer)[m_index]; } @@ -78,6 +96,9 @@ class static_circular_buffer { size_t m_index; }; + using reverse_iterator = std::reverse_iterator; + using const_reverse_iterator = std::reverse_iterator; + /** * Returns begin iterator. */ @@ -91,29 +112,67 @@ class static_circular_buffer { } /** - * Returns begin iterator. + * Returns const begin iterator. */ constexpr const_iterator begin() const { return const_iterator(this, 0); } /** - * Returns end iterator. + * Returns const end iterator. */ constexpr const_iterator end() const { return const_iterator(this, ::wpi::static_circular_buffer::size()); } /** - * Returns begin iterator. + * Returns const begin iterator. */ constexpr const_iterator cbegin() const { return const_iterator(this, 0); } /** - * Returns end iterator. + * Returns const end iterator. */ constexpr const_iterator cend() const { return const_iterator(this, ::wpi::static_circular_buffer::size()); } + /** + * Returns reverse begin iterator. + */ + constexpr reverse_iterator rbegin() { return reverse_iterator(end()); } + + /** + * Returns reverse end iterator. + */ + constexpr reverse_iterator rend() { return reverse_iterator(begin()); } + + /** + * Returns const reverse begin iterator. + */ + constexpr const_reverse_iterator rbegin() const { + return const_reverse_iterator(end()); + } + + /** + * Returns const reverse end iterator. + */ + constexpr const_reverse_iterator rend() const { + return const_reverse_iterator(begin()); + } + + /** + * Returns const reverse begin iterator. + */ + constexpr const_reverse_iterator crbegin() const { + return const_reverse_iterator(cend()); + } + + /** + * Returns const reverse end iterator. + */ + constexpr const_reverse_iterator crend() const { + return const_reverse_iterator(cbegin()); + } + /** * Returns number of elements in buffer */ diff --git a/wpiutil/src/test/native/cpp/CircularBufferTest.cpp b/wpiutil/src/test/native/cpp/CircularBufferTest.cpp index a56e2e5557..07f050d313 100644 --- a/wpiutil/src/test/native/cpp/CircularBufferTest.cpp +++ b/wpiutil/src/test/native/cpp/CircularBufferTest.cpp @@ -251,4 +251,18 @@ TEST(CircularBufferTest, Iterator) { EXPECT_EQ(values[i], elem); ++i; } + + // reverse_iterator + i = 2; + for (auto it = queue.rbegin(); it != queue.rend(); ++it) { + EXPECT_EQ(values[i], *it); + --i; + } + + // const_reverse_iterator + i = 2; + for (auto it = queue.crbegin(); it != queue.crend(); ++it) { + EXPECT_EQ(values[i], *it); + --i; + } } diff --git a/wpiutil/src/test/native/cpp/StaticCircularBufferTest.cpp b/wpiutil/src/test/native/cpp/StaticCircularBufferTest.cpp index 338bd8e239..23c7eb99d2 100644 --- a/wpiutil/src/test/native/cpp/StaticCircularBufferTest.cpp +++ b/wpiutil/src/test/native/cpp/StaticCircularBufferTest.cpp @@ -145,4 +145,18 @@ TEST(StaticCircularBufferTest, Iterator) { EXPECT_EQ(values[i], elem); ++i; } + + // reverse_iterator + i = 2; + for (auto it = queue.rbegin(); it != queue.rend(); ++it) { + EXPECT_EQ(values[i], *it); + --i; + } + + // const_reverse_iterator + i = 2; + for (auto it = queue.crbegin(); it != queue.crend(); ++it) { + EXPECT_EQ(values[i], *it); + --i; + } } From 2fb5271cc9be0d34292400a5ecfdf135ae9a10e6 Mon Sep 17 00:00:00 2001 From: sciencewhiz Date: Sun, 5 Oct 2025 14:22:53 -0700 Subject: [PATCH 10/10] [build] Update native-utils to 2026 (#8277) --- buildSrc/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/buildSrc/build.gradle b/buildSrc/build.gradle index 58b94ad4c4..2355d9bf92 100644 --- a/buildSrc/build.gradle +++ b/buildSrc/build.gradle @@ -9,5 +9,5 @@ repositories { } } dependencies { - implementation "edu.wpi.first:native-utils:2025.9.1" + implementation "edu.wpi.first:native-utils:2026.0.0" }