Aman-Mittal opened a new pull request, #171:
URL: https://github.com/apache/fineract-consumer-facing/pull/171
Closes #170.
## BFF (`consumer/build.gradle`)
The versions and settings match Fineract core.
| Tool | Version | Licence | Runs as |
|---|---|---|---|
| Error Prone (`net.ltgt.errorprone`) | 4.4.0 / core 2.49.0 | Apache-2.0 |
every `compileJava` |
| SpotBugs (`com.github.spotbugs`) +
`jp.skypencil.findbugs.slf4j:bug-pattern` | 6.5.4 / 1.5.0 | LGPL-2.1 (build
tool only, as in core) / Apache-2.0 | `spotbugsMain`, `spotbugsTest` |
| Modernizer (`com.github.andygoossens.modernizer`) | 1.13.0 | Apache-2.0 |
`modernizer` |
**Error Prone**
- Generated code (`build/generated/**`) is excluded.
- Core's disabled checks are disabled here too. `StringConcatToTextBlock` is
also disabled, because it crashes on JDK 25.
- Core's raised checks are errors here as well: `DefaultCharset`,
`StringSplitter`, `MutablePublicArray`, `EqualsGetClass` and
`FutureReturnValueIgnored`.
- About 25 warning-level findings remain. They don't fail the build and are
a follow-up.
**Exclusions**
- SpotBugs exclusions are in `consumer/config/spotbugs/exclude.xml`, each
with its reason:
- generated packages;
- core's project-wide `NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE` and
`EI_EXPOSE_REP*`;
- two single-class exclusions, explained in the file.
- Modernizer ignores the generated packages.
**CI**
- `build-tests-backend.yml` runs the analyzers after the clients are
generated. The test sources compile against the Java client generated from the
running stack, so the analyzers need the stack too.
- Reports are uploaded as `static-analysis-reports`.
## Frontend
The plugin set follows apache/fineract-backoffice-ui, which removed
`eslint-plugin-sonarjs` because it is LGPL-3.0-only (Category X):
| Plugin | Licence |
|---|---|
| `eslint-plugin-unicorn` 76, `configs.unopinionated` | MIT |
| `eslint-plugin-security` 4.0.1, `configs.recommended` | Apache-2.0 |
| `eslint-plugin-import-x` 4 + `eslint-import-resolver-typescript` | MIT /
ISC |
| local `eslint-rules/cognitive-complexity.js` (threshold 15), with tests |
ASF |
- **`import-x` instead of `eslint-plugin-import`:** the latter supports
ESLint only up to 9, and this repo is on ESLint 10.
- **New dependencies:** the 64 new transitive npm packages are all MIT, ISC,
BSD-2-Clause, CC0-1.0 or Apache-2.0.
- **Rules turned off:** each disabled rule has a comment beside it in
`eslint.config.js`. Examples:
- `unicorn/prefer-await`, for Angular's lazy-route `import().then`;
- `unicorn/no-array-sort`, since `toSorted` needs ES2023 and the target is
ES2022;
- `security/detect-object-injection`, for typed map indexing, as in the
back-office UI.
## Code changes made for the analyzers
Every change keeps behaviour the same.
**Java**
- `OpenBankingQueryServiceImpl`: a discarded `findFirst().orElseThrow(...)`
becomes `if (noneMatch(...)) throw ...`. The short-circuit and the exception
are the same.
- `AuditRetentionProperties` is now `final`. Nothing subclasses it, and
constructor binding works the same.
- In test code only, `getBytes()` becomes `getBytes(UTF_8)` (the strings are
ASCII). Four test call sites get a `@SuppressWarnings` with a reason, where
changing `split` would change behaviour.
**Frontend**
- `.replace(/…/g, …)` becomes `.replaceAll` in `pii-scrub.ts` and
`correlation-id.interceptor.ts`. Every pattern already has `g`.
- `(x * 16) | 0` becomes `Math.trunc` (the value is always in [0, 16)).
- `split(sep)` becomes `split(sep, n)` where only the first `n` parts are
used.
- `999999` becomes `999_999`, and `.find((v) => v)` becomes `.find(Boolean)`.
## Verification (local)
- `./gradlew compileJava compileTestJava spotbugsMain spotbugsTest
modernizer test` is green: 490 tests, 0 failures.
- `npm run lint`, `npm run format:check`, `npm run test:eslint-rules`
(26/26), `npm test` (125/125) and `npm run build` are green.
- The native build and the Docker image now also compile with Error Prone.
Locally they were not run, but CI on this PR covers both.
## Not in this PR
- **Spotless and Checkstyle:** they would reformat or flag most of the code,
so each is its own change.
- **SonarCloud:** this needs an ASF Infra-provisioned project and token.
**Overlap with #169:** that PR also adds `test:eslint-rules` (an ESLint rule
test) to `package.json` and to the lint workflow, and it edits
`eslint.config.js`. Whichever PR merges second will be rebased to combine them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
