Skip to content
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.

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

False positive no-semicolon #1933

Closed
nicolasamaduro opened this issue Apr 10, 2023 · 11 comments
Closed

False positive no-semicolon #1933

nicolasamaduro opened this issue Apr 10, 2023 · 11 comments

Comments

@nicolasamaduro
Copy link

In Kotlin, if you want to add properties or functions to an enum class, you have to separate their constants with a ; from the rest of the body (ref):

enum class Weekdays {
  MONDAY, TUESDAY, WEDNESDAY, THURSDAY, FRIDAY, SATURDAY, SUNDAY;

  fun isWeekend() = this == SATURDAY || this == SUNDAY
}

This semicolon is throwing a false positive no-semi rule and it also messes up with the trailing-comma-on-declaration-site rule.

@paul-dingemans
Copy link
Collaborator

The problem is not reproducible as too little information is given.

  • What is the ktlint version?
  • What are your .editorconfig settings?
  • What is the output produced?

You might also want to check whether your problem is reproducible with current snapshot versions in which some problems with enum have been resolved already.

@hfhbd
Copy link
Contributor

hfhbd commented Apr 11, 2023

With ktlint 0.48.2, no editorconfig and this gradle config we get the same error message:

package app.cash.sqldelight.core.integration

enum class Shoots {
  LEFT,
  RIGHT;

  enum class Type {
    ONE,
    TWO
  }
}
spotless {
  kotlin {
    target "**/*.kt"
    targetExclude "**/gen/**/*.*", "**/generated/**/*.*", "sqldelight-compiler/integration-tests/src/test/kotlin/com/example/**/*.*", "sqldelight-compiler/src/test/migration-interface-fixtures/**/*.*"
    ktlint("0.48.2").editorConfigOverride([
      "indent_size": "2",
      "ktlint_standard_package-name": "disabled",
      "ij_kotlin_allow_trailing_comma": "true",
      "ij_kotlin_allow_trailing_comma_on_call_site": "true",
    ])
    trimTrailingWhitespace()
    endWithNewline()
  }
}
Step 'ktlint' found problem in 'sqldelight-compiler/integration-tests/src/test/kotlin/app/cash/sqldelight/core/integration/Shoots.kt':
Error on line: 1, column: 1
rule: trailing-comma-on-declaration-site
Missing trailing comma before "}"
java.lang.AssertionError: Error on line: 1, column: 1
rule: trailing-comma-on-declaration-site
Missing trailing comma before "}"
	at com.diffplug.spotless.glue.ktlint.compat.KtLintCompatReporting.report(KtLintCompatReporting.java:23)
	at com.diffplug.spotless.glue.ktlint.compat.KtLintCompat0Dot48Dot0Adapter$FormatterCallback.invoke(KtLintCompat0Dot48Dot0Adapter.java:74)
	at com.diffplug.spotless.glue.ktlint.compat.KtLintCompat0Dot48Dot0Adapter$FormatterCallback.invoke(KtLintCompat0Dot48Dot0Adapter.java:70)
	at com.pinterest.ktlint.core.KtLintRuleEngine.format(KtLint.kt:569)
	at com.pinterest.ktlint.core.KtLintRuleEngine.format(KtLint.kt:485)
	at com.diffplug.spotless.glue.ktlint.compat.KtLintCompat0Dot48Dot0Adapter.format(KtLintCompat0Dot48Dot0Adapter.java:114)
	at com.diffplug.spotless.glue.ktlint.KtlintFormatterFunc.applyWithFile(KtlintFormatterFunc.java:65)
	at com.diffplug.spotless.FormatterFunc$NeedsFile.apply(FormatterFunc.java:154)
	at com.diffplug.spotless.FormatterStepImpl$Standard.format(FormatterStepImpl.java:82)
	at com.diffplug.spotless.FormatterStep$Strict.format(FormatterStep.java:88)
	at com.diffplug.spotless.Formatter.compute(Formatter.java:246)
	at com.diffplug.spotless.PaddedCell.calculateDirtyState(PaddedCell.java:203)
	at com.diffplug.spotless.PaddedCell.calculateDirtyState(PaddedCell.java:190)
	at com.diffplug.gradle.spotless.SpotlessTaskImpl.processInputFile(SpotlessTaskImpl.java:105)
	at com.diffplug.gradle.spotless.SpotlessTaskImpl.performAction(SpotlessTaskImpl.java:89)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at org.gradle.internal.reflect.JavaMethod.invoke(JavaMethod.java:125)
	at org.gradle.api.internal.project.taskfactory.IncrementalTaskAction.doExecute(IncrementalTaskAction.java:45)
	at org.gradle.api.internal.project.taskfactory.StandardTaskAction.execute(StandardTaskAction.java:51)
	at org.gradle.api.internal.project.taskfactory.IncrementalTaskAction.execute(IncrementalTaskAction.java:26)
	at org.gradle.api.internal.project.taskfactory.StandardTaskAction.execute(StandardTaskAction.java:29)
	at org.gradle.api.internal.tasks.execution.TaskExecution$3.run(TaskExecution.java:242)
	at org.gradle.internal.operations.DefaultBuildOperationRunner$1.execute(DefaultBuildOperationRunner.java:29)
	at org.gradle.internal.operations.DefaultBuildOperationRunner$1.execute(DefaultBuildOperationRunner.java:26)
	at org.gradle.internal.operations.DefaultBuildOperationRunner$2.execute(DefaultBuildOperationRunner.java:66)
	at org.gradle.internal.operations.DefaultBuildOperationRunner$2.execute(DefaultBuildOperationRunner.java:59)
	at org.gradle.internal.operations.DefaultBuildOperationRunner.execute(DefaultBuildOperationRunner.java:157)
	at org.gradle.internal.operations.DefaultBuildOperationRunner.execute(DefaultBuildOperationRunner.java:59)
	at org.gradle.internal.operations.DefaultBuildOperationRunner.run(DefaultBuildOperationRunner.java:47)
	at org.gradle.internal.operations.DefaultBuildOperationExecutor.run(DefaultBuildOperationExecutor.java:68)
	at org.gradle.api.internal.tasks.execution.TaskExecution.executeAction(TaskExecution.java:227)
	at org.gradle.api.internal.tasks.execution.TaskExecution.executeActions(TaskExecution.java:210)
	at org.gradle.api.internal.tasks.execution.TaskExecution.executeWithPreviousOutputFiles(TaskExecution.java:193)
	at org.gradle.api.internal.tasks.execution.TaskExecution.execute(TaskExecution.java:166)
	at org.gradle.internal.execution.steps.ExecuteStep.executeInternal(ExecuteStep.java:93)
	at org.gradle.internal.execution.steps.ExecuteStep.access$000(ExecuteStep.java:44)
	at org.gradle.internal.execution.steps.ExecuteStep$1.call(ExecuteStep.java:57)
	at org.gradle.internal.execution.steps.ExecuteStep$1.call(ExecuteStep.java:54)
	at org.gradle.internal.operations.DefaultBuildOperationRunner$CallableBuildOperationWorker.execute(DefaultBuildOperationRunner.java:204)
	at org.gradle.internal.operations.DefaultBuildOperationRunner$CallableBuildOperationWorker.execute(DefaultBuildOperationRunner.java:199)
	at org.gradle.internal.operations.DefaultBuildOperationRunner$2.execute(DefaultBuildOperationRunner.java:66)
	at org.gradle.internal.operations.DefaultBuildOperationRunner$2.execute(DefaultBuildOperationRunner.java:59)
	at org.gradle.internal.operations.DefaultBuildOperationRunner.execute(DefaultBuildOperationRunner.java:157)
	at org.gradle.internal.operations.DefaultBuildOperationRunner.execute(DefaultBuildOperationRunner.java:59)
	at org.gradle.internal.operations.DefaultBuildOperationRunner.call(DefaultBuildOperationRunner.java:53)
	at org.gradle.internal.operations.DefaultBuildOperationExecutor.call(DefaultBuildOperationExecutor.java:73)
	at org.gradle.internal.execution.steps.ExecuteStep.execute(ExecuteStep.java:54)
	at org.gradle.internal.execution.steps.ExecuteStep.execute(ExecuteStep.java:44)
	at org.gradle.internal.execution.steps.RemovePreviousOutputsStep.execute(RemovePreviousOutputsStep.java:67)
	at org.gradle.internal.execution.steps.RemovePreviousOutputsStep.execute(RemovePreviousOutputsStep.java:37)
	at org.gradle.internal.execution.steps.CancelExecutionStep.execute(CancelExecutionStep.java:41)
	at org.gradle.internal.execution.steps.TimeoutStep.executeWithoutTimeout(TimeoutStep.java:74)
	at org.gradle.internal.execution.steps.TimeoutStep.execute(TimeoutStep.java:55)
	at org.gradle.internal.execution.steps.CreateOutputsStep.execute(CreateOutputsStep.java:50)
	at org.gradle.internal.execution.steps.CreateOutputsStep.execute(CreateOutputsStep.java:28)
	at org.gradle.internal.execution.steps.CaptureStateAfterExecutionStep.executeDelegateBroadcastingChanges(CaptureStateAfterExecutionStep.java:100)
	at org.gradle.internal.execution.steps.CaptureStateAfterExecutionStep.execute(CaptureStateAfterExecutionStep.java:72)
	at org.gradle.internal.execution.steps.CaptureStateAfterExecutionStep.execute(CaptureStateAfterExecutionStep.java:50)
	at org.gradle.internal.execution.steps.ResolveInputChangesStep.execute(ResolveInputChangesStep.java:40)
	at org.gradle.internal.execution.steps.ResolveInputChangesStep.execute(ResolveInputChangesStep.java:29)
	at org.gradle.internal.execution.steps.BuildCacheStep.executeWithoutCache(BuildCacheStep.java:166)
	at org.gradle.internal.execution.steps.BuildCacheStep.lambda$execute$1(BuildCacheStep.java:70)
	at org.gradle.internal.Either$Right.fold(Either.java:175)
	at org.gradle.internal.execution.caching.CachingState.fold(CachingState.java:59)
	at org.gradle.internal.execution.steps.BuildCacheStep.execute(BuildCacheStep.java:68)
	at org.gradle.internal.execution.steps.BuildCacheStep.execute(BuildCacheStep.java:46)
	at org.gradle.internal.execution.steps.StoreExecutionStateStep.execute(StoreExecutionStateStep.java:36)
	at org.gradle.internal.execution.steps.StoreExecutionStateStep.execute(StoreExecutionStateStep.java:25)
	at org.gradle.internal.execution.steps.RecordOutputsStep.execute(RecordOutputsStep.java:36)
	at org.gradle.internal.execution.steps.RecordOutputsStep.execute(RecordOutputsStep.java:22)
	at org.gradle.internal.execution.steps.SkipUpToDateStep.executeBecause(SkipUpToDateStep.java:91)
	at org.gradle.internal.execution.steps.SkipUpToDateStep.lambda$execute$2(SkipUpToDateStep.java:55)
	at org.gradle.internal.execution.steps.SkipUpToDateStep.execute(SkipUpToDateStep.java:55)
	at org.gradle.internal.execution.steps.SkipUpToDateStep.execute(SkipUpToDateStep.java:37)
	at org.gradle.internal.execution.steps.ResolveChangesStep.execute(ResolveChangesStep.java:65)
	at org.gradle.internal.execution.steps.ResolveChangesStep.execute(ResolveChangesStep.java:36)
	at org.gradle.internal.execution.steps.legacy.MarkSnapshottingInputsFinishedStep.execute(MarkSnapshottingInputsFinishedStep.java:37)
	at org.gradle.internal.execution.steps.legacy.MarkSnapshottingInputsFinishedStep.execute(MarkSnapshottingInputsFinishedStep.java:27)
	at org.gradle.internal.execution.steps.ResolveCachingStateStep.execute(ResolveCachingStateStep.java:76)
	at org.gradle.internal.execution.steps.ResolveCachingStateStep.execute(ResolveCachingStateStep.java:37)
	at org.gradle.internal.execution.steps.ValidateStep.execute(ValidateStep.java:94)
	at org.gradle.internal.execution.steps.ValidateStep.execute(ValidateStep.java:49)
	at org.gradle.internal.execution.steps.CaptureStateBeforeExecutionStep.execute(CaptureStateBeforeExecutionStep.java:71)
	at org.gradle.internal.execution.steps.CaptureStateBeforeExecutionStep.execute(CaptureStateBeforeExecutionStep.java:45)
	at org.gradle.internal.execution.steps.SkipEmptyWorkStep.executeWithNonEmptySources(SkipEmptyWorkStep.java:177)
	at org.gradle.internal.execution.steps.SkipEmptyWorkStep.execute(SkipEmptyWorkStep.java:81)
	at org.gradle.internal.execution.steps.SkipEmptyWorkStep.execute(SkipEmptyWorkStep.java:53)
	at org.gradle.internal.execution.steps.RemoveUntrackedExecutionStateStep.execute(RemoveUntrackedExecutionStateStep.java:32)
	at org.gradle.internal.execution.steps.RemoveUntrackedExecutionStateStep.execute(RemoveUntrackedExecutionStateStep.java:21)
	at org.gradle.internal.execution.steps.legacy.MarkSnapshottingInputsStartedStep.execute(MarkSnapshottingInputsStartedStep.java:38)
	at org.gradle.internal.execution.steps.LoadPreviousExecutionStateStep.execute(LoadPreviousExecutionStateStep.java:36)
	at org.gradle.internal.execution.steps.LoadPreviousExecutionStateStep.execute(LoadPreviousExecutionStateStep.java:23)
	at org.gradle.internal.execution.steps.CleanupStaleOutputsStep.execute(CleanupStaleOutputsStep.java:75)
	at org.gradle.internal.execution.steps.CleanupStaleOutputsStep.execute(CleanupStaleOutputsStep.java:41)
	at org.gradle.internal.execution.steps.AssignWorkspaceStep.lambda$execute$0(AssignWorkspaceStep.java:32)
	at org.gradle.api.internal.tasks.execution.TaskExecution$4.withWorkspace(TaskExecution.java:287)
	at org.gradle.internal.execution.steps.AssignWorkspaceStep.execute(AssignWorkspaceStep.java:30)
	at org.gradle.internal.execution.steps.AssignWorkspaceStep.execute(AssignWorkspaceStep.java:21)
	at org.gradle.internal.execution.steps.IdentityCacheStep.execute(IdentityCacheStep.java:37)
	at org.gradle.internal.execution.steps.IdentityCacheStep.execute(IdentityCacheStep.java:27)
	at org.gradle.internal.execution.steps.IdentifyStep.execute(IdentifyStep.java:42)
	at org.gradle.internal.execution.steps.IdentifyStep.execute(IdentifyStep.java:31)
	at org.gradle.internal.execution.impl.DefaultExecutionEngine$1.execute(DefaultExecutionEngine.java:64)
	at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.executeIfValid(ExecuteActionsTaskExecuter.java:146)
	at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.execute(ExecuteActionsTaskExecuter.java:135)
	at org.gradle.api.internal.tasks.execution.FinalizePropertiesTaskExecuter.execute(FinalizePropertiesTaskExecuter.java:46)
	at org.gradle.api.internal.tasks.execution.ResolveTaskExecutionModeExecuter.execute(ResolveTaskExecutionModeExecuter.java:51)
	at org.gradle.api.internal.tasks.execution.SkipTaskWithNoActionsExecuter.execute(SkipTaskWithNoActionsExecuter.java:57)
	at org.gradle.api.internal.tasks.execution.SkipOnlyIfTaskExecuter.execute(SkipOnlyIfTaskExecuter.java:74)
	at org.gradle.api.internal.tasks.execution.CatchExceptionTaskExecuter.execute(CatchExceptionTaskExecuter.java:36)
	at org.gradle.api.internal.tasks.execution.EventFiringTaskExecuter$1.executeTask(EventFiringTaskExecuter.java:77)
	at org.gradle.api.internal.tasks.execution.EventFiringTaskExecuter$1.call(EventFiringTaskExecuter.java:55)
	at org.gradle.api.internal.tasks.execution.EventFiringTaskExecuter$1.call(EventFiringTaskExecuter.java:52)
	at org.gradle.internal.operations.DefaultBuildOperationRunner$CallableBuildOperationWorker.execute(DefaultBuildOperationRunner.java:204)
	at org.gradle.internal.operations.DefaultBuildOperationRunner$CallableBuildOperationWorker.execute(DefaultBuildOperationRunner.java:199)
	at org.gradle.internal.operations.DefaultBuildOperationRunner$2.execute(DefaultBuildOperationRunner.java:66)
	at org.gradle.internal.operations.DefaultBuildOperationRunner$2.execute(DefaultBuildOperationRunner.java:59)
	at org.gradle.internal.operations.DefaultBuildOperationRunner.execute(DefaultBuildOperationRunner.java:157)
	at org.gradle.internal.operations.DefaultBuildOperationRunner.execute(DefaultBuildOperationRunner.java:59)
	at org.gradle.internal.operations.DefaultBuildOperationRunner.call(DefaultBuildOperationRunner.java:53)
	at org.gradle.internal.operations.DefaultBuildOperationExecutor.call(DefaultBuildOperationExecutor.java:73)
	at org.gradle.api.internal.tasks.execution.EventFiringTaskExecuter.execute(EventFiringTaskExecuter.java:52)
	at org.gradle.execution.plan.LocalTaskNodeExecutor.execute(LocalTaskNodeExecutor.java:42)
	at org.gradle.execution.taskgraph.DefaultTaskExecutionGraph$InvokeNodeExecutorsAction.execute(DefaultTaskExecutionGraph.java:338)
	at org.gradle.execution.taskgraph.DefaultTaskExecutionGraph$InvokeNodeExecutorsAction.execute(DefaultTaskExecutionGraph.java:325)
	at org.gradle.execution.taskgraph.DefaultTaskExecutionGraph$BuildOperationAwareExecutionAction.execute(DefaultTaskExecutionGraph.java:318)
	at org.gradle.execution.taskgraph.DefaultTaskExecutionGraph$BuildOperationAwareExecutionAction.execute(DefaultTaskExecutionGraph.java:304)
	at org.gradle.execution.plan.DefaultPlanExecutor$ExecutorWorker.execute(DefaultPlanExecutor.java:463)
	at org.gradle.execution.plan.DefaultPlanExecutor$ExecutorWorker.run(DefaultPlanExecutor.java:380)
	at org.gradle.execution.plan.DefaultPlanExecutor.process(DefaultPlanExecutor.java:116)
	at org.gradle.execution.taskgraph.DefaultTaskExecutionGraph.executeWithServices(DefaultTaskExecutionGraph.java:136)
	at org.gradle.execution.taskgraph.DefaultTaskExecutionGraph.execute(DefaultTaskExecutionGraph.java:121)
	at org.gradle.execution.SelectedTaskExecutionAction.execute(SelectedTaskExecutionAction.java:35)
	at org.gradle.execution.DryRunBuildExecutionAction.execute(DryRunBuildExecutionAction.java:51)
	at org.gradle.execution.BuildOperationFiringBuildWorkerExecutor$ExecuteTasks.call(BuildOperationFiringBuildWorkerExecutor.java:54)
	at org.gradle.execution.BuildOperationFiringBuildWorkerExecutor$ExecuteTasks.call(BuildOperationFiringBuildWorkerExecutor.java:43)
	at org.gradle.internal.operations.DefaultBuildOperationRunner$CallableBuildOperationWorker.execute(DefaultBuildOperationRunner.java:204)
	at org.gradle.internal.operations.DefaultBuildOperationRunner$CallableBuildOperationWorker.execute(DefaultBuildOperationRunner.java:199)
	at org.gradle.internal.operations.DefaultBuildOperationRunner$2.execute(DefaultBuildOperationRunner.java:66)
	at org.gradle.internal.operations.DefaultBuildOperationRunner$2.execute(DefaultBuildOperationRunner.java:59)
	at org.gradle.internal.operations.DefaultBuildOperationRunner.execute(DefaultBuildOperationRunner.java:157)
	at org.gradle.internal.operations.DefaultBuildOperationRunner.execute(DefaultBuildOperationRunner.java:59)
	at org.gradle.internal.operations.DefaultBuildOperationRunner.call(DefaultBuildOperationRunner.java:53)
	at org.gradle.internal.operations.DefaultBuildOperationExecutor.call(DefaultBuildOperationExecutor.java:73)
	at org.gradle.execution.BuildOperationFiringBuildWorkerExecutor.execute(BuildOperationFiringBuildWorkerExecutor.java:40)
	at org.gradle.internal.build.DefaultBuildLifecycleController.lambda$executeTasks$7(DefaultBuildLifecycleController.java:172)
	at org.gradle.internal.model.StateTransitionController.doTransition(StateTransitionController.java:258)
	at org.gradle.internal.model.StateTransitionController.lambda$tryTransition$8(StateTransitionController.java:185)
	at org.gradle.internal.work.DefaultSynchronizer.withLock(DefaultSynchronizer.java:44)
	at org.gradle.internal.model.StateTransitionController.tryTransition(StateTransitionController.java:185)
	at org.gradle.internal.build.DefaultBuildLifecycleController.executeTasks(DefaultBuildLifecycleController.java:172)
	at org.gradle.internal.build.DefaultBuildWorkGraphController$DefaultBuildWorkGraph.runWork(DefaultBuildWorkGraphController.java:209)
	at org.gradle.internal.work.DefaultWorkerLeaseService.withLocks(DefaultWorkerLeaseService.java:249)
	at org.gradle.internal.work.DefaultWorkerLeaseService.runAsWorkerThread(DefaultWorkerLeaseService.java:109)
	at org.gradle.composite.internal.DefaultBuildController.doRun(DefaultBuildController.java:172)
	at org.gradle.composite.internal.DefaultBuildController.access$000(DefaultBuildController.java:47)
	at org.gradle.composite.internal.DefaultBuildController$BuildOpRunnable.run(DefaultBuildController.java:191)
	at org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:64)
	at org.gradle.internal.concurrent.ManagedExecutorImpl$1.run(ManagedExecutorImpl.java:49)

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':spotlessKotlin'.
> Error on line: 1, column: 1
  rule: trailing-comma-on-declaration-site
  Missing trailing comma before "}"

@hfhbd
Copy link
Contributor

hfhbd commented Apr 11, 2023

At least our problem was fixed with this kotlin code:

package app.cash.sqldelight.core.integration

enum class Shoots {
  LEFT,
  RIGHT, // added manually
  ;

  enum class Type {
    ONE,
    TWO,
  }
}

While I absolutely understand the new trailing comma, the error message is hard to debug.

@paul-dingemans
Copy link
Collaborator

  • What went wrong:
    Execution failed for task ':spotlessKotlin'.

Error on line: 1, column: 1
rule: trailing-comma-on-declaration-site
Missing trailing comma before "}"

The output above is produced by Spotless. The Ktlint CLI (lint only) produces output like below:

src/main/kotlin/Foo.kt:5:10: Missing trailing comma before ";" (trailing-comma-on-declaration-site)
src/main/kotlin/Foo.kt:9:12: Missing trailing comma before "}" (trailing-comma-on-declaration-site)

which contains both the line number as well as the offset on the line.

When running Ktlint CLI with formatting, the output below is produced:

src/main/kotlin/Foo.kt:1:1: Missing trailing comma before "}" (trailing-comma-on-declaration-site)
src/main/kotlin/Foo.kt:7:4: Unnecessary semicolon (no-semi)

which indeed is not correct and already has been fixed in the snapshot of the upcoming 0.49.x release. 0.48.2 however did format the code correctly.

@paul-dingemans paul-dingemans closed this as not planned Won't fix, can't repro, duplicate, stale Apr 11, 2023
@stkent
Copy link

stkent commented May 11, 2023

I ran into this same case, I think, and filed an issue on the Spotless repo. diffplug/spotless#1699

Should ktlint -F be exiting with a non-zero exit code when it successfully reformats source code? (I am not familiar enough with standalone ktlint to know the norms around its exit codes.) e.g. when I run ktlint -F (version 0.48.2) on this file:

enum class AccountType {
    Case1,
    Case2;

    companion object {
    }
}

is it expected that both (1) the code is successfully reformatted (see below) and (2) the exit code is 1?

enum class AccountType {
    Case1,
    Case2,
    ;

    companion object {
    }
}

Thanks in advance for any light you can shed on this!

@paul-dingemans
Copy link
Collaborator

In case all lint errors have been autocorrected, ktlint -F returns with exit code 0. In case at least one error could not be autocorrected, ktlint -F returns exit code 1. Note that ktlint (without -F) returns exit code 1 whenever a lint error is found regardless whether it can or cannot be autocorrected.

@stkent
Copy link

stkent commented May 11, 2023

Thanks for the clarification for expected behavior, @paul-dingemans! Given this information, this sequence of events looks like it shows a bug?

  • Initial source file:
    enum class AccountType {
        Case1,
        Case2;
    
        companion object {
        }
    }
  • Run ktlint -F path/to/AccountType.kt.
    • Output:
      .../AccountType.kt:1:1: Missing trailing comma before "}" (trailing-comma-on-declaration-site)
      .../AccountType.kt:5:4: Unnecessary semicolon (no-semi)
      
      Summary error count (descending) by rule:
        no-semi: 1
        trailing-comma-on-declaration-site: 1
      
    • Exit code is 1, implying that at least one error could not be autocorrected.
    • Updated source file:
      enum class AccountType {
          Case1,
          Case2,
          ;
      
          companion object {
          }
      }
  • Run ktlint -F path/to/AccountType.kt again.
    • Source file is unchanged.
    • Since the previous invocation of ktlint indicated that at least one error could not be autocorrected, this invocation should also produce exit code 1. But it does not; the exit code is 0. This contradicts the assertion that a non-correctable error exists in the updated source file.

Perhaps it is the case that in fixing a correctable error, the error that could not (in isolation) be autocorrected is "accidentally" also fixed?

@paul-dingemans
Copy link
Collaborator

Run ktlint -F path/to/AccountType.kt.

  • Output:
    .../AccountType.kt:1:1: Missing trailing comma before "}" (trailing-comma-on-declaration-site)
    .../AccountType.kt:5:4: Unnecessary semicolon (no-semi)
    
    Summary error count (descending) by rule:
      no-semi: 1
      trailing-comma-on-declaration-site: 1
    

Are you sure that you actually did run with -F? If so, I would not have expected to see the lint violation being reported. For me the command ktlint -F path/to/AccountType.kt returns 0. Without -F I do see the output you mentioned and the return code is indeed 1.

@stkent
Copy link

stkent commented May 11, 2023

I believe that I did!

CleanShot 2023-05-11 at 15 15 53@2x

Note (in case this note was not obvious in my original comment) that this is with ktlint 0.48.2. I see the same as you with ktlint 0.49.0, but upgrading to that version is not possible for our project yet (we rely on a Gradle plugin that has not updated with 0.49.0 support). I now see that this is what you were referencing in #1933 (comment); the formatting is applied correctly despite the non-zero exit code.

Is there the possibility for a 0.48.3 release that includes this fix? If not we will keep an eye out for Spotless to support 0.49.0 (diffplug/spotless#1696).

@paul-dingemans
Copy link
Collaborator

Is there the possibility for a 0.48.3 release that includes this fix? If not we will keep an eye out for Spotless to support 0.49.0 (diffplug/spotless#1696).

Sorry, but 0.48 will not be fixed for this.

@stkent
Copy link

stkent commented May 12, 2023

Is there the possibility for a 0.48.3 release that includes this fix? If not we will keep an eye out for Spotless to support 0.49.0 (diffplug/spotless#1696).

Sorry, but 0.48 will not be fixed for this.

No worries, thanks for confirming :) Appreciate the effort going into reaching 1.0!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants