andygrove opened a new pull request, #5175: URL: https://github.com/apache/datafusion-comet/pull/5175
## Which issue does this PR close? No issue filed — mechanical compiler-warning cleanup, split out of #5141. Follows #5168, #5170 and #5173. ## Rationale for this change Scala 2.13 deprecates *auto-application*: writing `foo` where the method is declared `def foo()`. Comet does this at 20 sites — the largest remaining group of warnings under the 2.13 profiles. Auto-application is purely syntactic: the method is already being invoked, so adding `()` cannot change behaviour or evaluation order. That makes 19 of these a genuine one-token fix. The 20th is the interesting one, and it is a declaration rather than a call. ## What changes are included in this PR? **`()` added at the call site (19 sites, 19 warnings):** - `SparkSession.builder` → `builder()` in nine benchmark/TPC session builders. - `Random.nextLong` / `Random.nextInt` in `CometBenchmarkBase` (×2) and `CometReadBenchmark`. - `Dataset.count` in `CometNativeReaderSuite` and `Tables`. - `TaskContext.get().taskAttemptId` in `CometExecIterator` — the very next line already reads `TaskContext.get().cpus()`. - `TimestampFormatter.defaultPattern` in `unixtime.scala` (×2). - `CometConf.COMET_SHUFFLE_COMPRESSION_ZSTD_LEVEL.get` in `CometNativeShuffleWriter` — the following two lines already read `.get()`. **`NativeBatchDecoderIterator` (2 warnings, 1 fix):** it declares `def hasNext(): Boolean` while `Iterator` declares `hasNext` *without* a parameter list. That mismatch warns twice — once on the declaration (*"method with a single empty parameter list overrides method hasNext in trait Iterator defined without a parameter list"*) and again on the internal `if (!hasNext)` call, which is then an auto-application. Dropping the parameter list from the override fixes both. This is the one change here that could in principle break a caller, so I checked: the only consumer is `CometBlockStoreShuffleReader`, which uses the iterator through `Iterator.flatMap` and never calls `hasNext` directly. Java callers would be unaffected either way — the JVM signature is unchanged. `spotless:apply` reflows the nine `SparkSession.builder()` calls onto their own line (`SparkSession` / `.builder()`), since the extra parens push the chain over scalafmt's threshold. That is the only formatting change in the diff. ## How are these changes tested? No new tests — no intended behavior change. - Warnings under the default profile (Spark 4.1 / Scala 2.13): **127 → 106**, all 21 gone and none added, verified by diffing the full sorted warning list before and after. - Scala 2.12 (`-Pspark-3.5`): warning list **byte-identical** (15 → 15). Auto-application is not deprecated on 2.12, and `Iterator.hasNext` is parameterless there too. - `test-compile` passes on all five profiles: default, `-Pspark-3.4`, `-Pspark-3.5`, `-Pspark-3.5 -Pscala-2.13`, `-Pspark-4.0`. - `spotless:check` and `scalastyle:check` pass. - 265 tests pass across the suites covering the four main-source changes: `CometNativeShuffleSuite` and `CometShuffleSuite` (the `NativeBatchDecoderIterator` read path and the shuffle writer's compression level), `CometNativeReaderSuite`, and `CometExpressionSuite` (`from_unixtime`, both the default-pattern and non-default-pattern branches). `CometExecIterator` is exercised by every one of them. - The benchmark files are compile-only in CI; they are covered by `test-compile` above. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
