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

Formatting with no-semi rule removes semicolons in enum, even if continues companion object #1733

Closed
muniere opened this issue Dec 16, 2022 · 10 comments · Fixed by #1736
Closed
Milestone

Comments

@muniere
Copy link

muniere commented Dec 16, 2022

Expected Behavior

Keep the following code as it is.

enum class Color(val rawValue: Int) {
    RED(1),
    GREEN(2);

    companion object {
        val allValues = listOf(RED, GREEN)
    }
}

Observed Behavior

With 0.48.0, using --format option removes the semicolon, and it is not a valid Kotlin code.

enum class Color(val rawValue: Int) {
    RED(1),
    GREEN(2)

    companion object {
        val allValues = listOf(RED, GREEN)
    }
}

Steps to Reproduce

Run the following command:

$ ktlint --format src/**/*.kt android --disabled_rules=wrapping

When disabling no-semi rule, semicolon is not removed.

Your Environment

  • Version of ktlint used: 0.48.0
  • Version of Gradle used: 7.5.1
  • Operating System and version: macOS Monterey 12.6
3flex added a commit to 3flex/detekt that referenced this issue Dec 16, 2022
This can be enabled again when this defect is fixed:
pinterest/ktlint#1733
@josephlbarnett
Copy link

similar behavior if there are methods in the enum after the value definitions, not limited to companion objects

@paul-dingemans
Copy link
Collaborator

paul-dingemans commented Dec 16, 2022

Tnx for reporting. This will be fixed in 0.48.1. For now you can suppress the no-semi rule as follows:

@Suppress("ktlint:no-semi") // Remove when fixed in ktlint 0.48.1
enum class Color(val rawValue: Int) {
    RED(1),
    GREEN(2);

    companion object {
        val allValues = listOf(RED, GREEN)
    }
}

Or maybe better, suppress the rule temporarily in .editorconfig with 'ktlint_no-semi = disabled`.

@paul-dingemans paul-dingemans added this to the 0.48.1 milestone Dec 16, 2022
@paul-dingemans paul-dingemans pinned this issue Dec 17, 2022
paul-dingemans added a commit to paul-dingemans/ktlint that referenced this issue Dec 17, 2022
…st of enum entries is followed by a semicolon then do not remove the semicolon in case it is followed by code element

Closes pinterest#1733
3flex added a commit to detekt/detekt that referenced this issue Dec 17, 2022
* Update ktlint to v0.48.0

* Update imports for ktlint 0.48.0

* Update finding locations for ktlint 0.48.0

* Disable ktlint's NoSemicolons rule

This can be enabled again when this defect is fixed:
pinterest/ktlint#1733

* Fix new indentation issues raised by ktlint 0.48.0

* Add ContextReceiverMapping rule

* Enable ContextReceiverMapping rule on detekt codebase

* Fix deprecation warnings
paul-dingemans added a commit that referenced this issue Dec 18, 2022
…ructor (#1736)

An enumeration class having a primary constructor and in which the list of enum entries is followed by a semicolon then do not remove the semicolon in case it is followed by code element

Closes #1733
@paul-dingemans
Copy link
Collaborator

A snapshot release including a fix is available. Read more on https://pinterest.github.io/ktlint/install/snapshot-build/

@muniere
Copy link
Author

muniere commented Dec 20, 2022

Thanks for quick fix! 👍

@blommish
Copy link

blommish commented Jan 2, 2023

Tnx for reporting. This will be fixed in 0.48.1. For now you can suppress the no-semi rule as follows:

@Suppress("ktlint:no-semi") // Remove when fixed in ktlint 0.48.1
enum class Color(val rawValue: Int) {
    RED(1),
    GREEN(2);

    companion object {
        val allValues = listOf(RED, GREEN)
    }
}

Or maybe better, suppress the rule temporarily in .editorconfig with 'ktlint_no-semi = disabled`.

ktlint --format src/**/*.kt

@paul-dingemans
With this, it adds a coma to the last entry, moving the semi-colon one line down.

Missing trailing comma before "}" (trailing-comma-on-declaration-site)

enum class Foo(val string: String) {
    FOO("val1"),
    FOO2("val1"),
    ;
    
    fun any() = ""

@paul-dingemans
Copy link
Collaborator

@paul-dingemans With this, it adds a coma to the last entry, moving the semi-colon one line down.

Missing trailing comma before "}" (trailing-comma-on-declaration-site)

enum class Foo(val string: String) {
    FOO("val1"),
    FOO2("val1"),
    ;
    
    fun any() = ""

Yes, that is expected behavior. When adding a new enum entry after "FOO2" you don't want that line to appear in the git diff because a comma needs to be added on the line.

@AlvaroBrey
Copy link

AlvaroBrey commented Jan 7, 2023

Yes, that is expected behavior. When adding a new enum entry after "FOO2" you don't want that line to appear in the git diff because a comma needs to be added on the line.

Any way to keep the previous behaviour other than suppressing rules on a per-file basis? Meaning this being a "correct" format:

enum class Foo(val string: String) {
    FOO("val1"),
    FOO2("val1");
    
    fun any() = ""

@paul-dingemans
Copy link
Collaborator

There is no such option. If you have set trailing comma's, this is what you get. Also, no such option will be made available in the future.

@Maragues
Copy link

Maragues commented Mar 10, 2023

Hi, I'm seeing this same problem using 0.48.2. Should I open a separate issue?

enum class FileType(val extension: String) {
    PB("pb"),
    PBTXT("pbtxt");

    fun extensionSupported(extension: String): Boolean = this.extension == extension
}

fun File.isPb(): Boolean = FileType.PB.extensionSupported(extension.lowercase())

In order to pass spotlessCheck, I've manually had to change to this

@Suppress("ktlint:no-semi")
internal enum class FileType(val extension: String) {
    PB("pb"),
    PBTXT("pbtxt"),
    ;

    fun extensionSupported(extension: String): Boolean {
        return this.extension == extension
    }
}

fun File.isPb(): Boolean = FileType.PB.extensionSupported(extension.lowercase())

Otherwise, I was getting weird errors at the declaration of the extension function

Step 'ktlint' found problem in 'src/main/java/FileType.kt':
Error on line: 26, column: 3
rule: trailing-comma-on-declaration-site
Missing trailing comma before "}"
java.lang.AssertionError: Error on line: 26, column: 3
rule: trailing-comma-on-declaration-site
Missing trailing comma before "}"

This is my spotless configuration, in case it's relevant

spotless {
    java {
        ...
    }

    kotlin {
        def disabledRules = ["disabled_rules": "filename"]
        ktlint("0.48.2").userData(disabledRules)

        target '**/*.kt'
    }
}
Error on line: 19, column: 4
rule: no-semi
Unnecessary semicolon
java.lang.AssertionError: Error on line: 19, column: 4
rule: no-semi
Unnecessary semicolon
        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:102)
        at com.diffplug.gradle.spotless.SpotlessTaskImpl.performAction(SpotlessTaskImpl.java:88)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        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.IncrementalInputsTaskAction.doExecute(IncrementalInputsTaskAction.java:32)
        at org.gradle.api.internal.project.taskfactory.StandardTaskAction.execute(StandardTaskAction.java:51)
        at org.gradle.api.internal.project.taskfactory.AbstractIncrementalTaskAction.execute(AbstractIncrementalTaskAction.java:25)
        at org.gradle.api.internal.project.taskfactory.StandardTaskAction.execute(StandardTaskAction.java:29)
        at org.gradle.api.internal.tasks.execution.TaskExecution$3.run(TaskExecution.java:236)
        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:221)
        at org.gradle.api.internal.tasks.execution.TaskExecution.executeActions(TaskExecution.java:204)
        at org.gradle.api.internal.tasks.execution.TaskExecution.executeWithPreviousOutputFiles(TaskExecution.java:187)
        at org.gradle.api.internal.tasks.execution.TaskExecution.execute(TaskExecution.java:165)
        at org.gradle.internal.execution.steps.ExecuteStep.executeInternal(ExecuteStep.java:89)
        at org.gradle.internal.execution.steps.ExecuteStep.access$000(ExecuteStep.java:40)
        at org.gradle.internal.execution.steps.ExecuteStep$1.call(ExecuteStep.java:53)
        at org.gradle.internal.execution.steps.ExecuteStep$1.call(ExecuteStep.java:50)
        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:50)
        at org.gradle.internal.execution.steps.ExecuteStep.execute(ExecuteStep.java:40)
        at org.gradle.internal.execution.steps.RemovePreviousOutputsStep.execute(RemovePreviousOutputsStep.java:68)
        at org.gradle.internal.execution.steps.RemovePreviousOutputsStep.execute(RemovePreviousOutputsStep.java:38)
        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:51)
        at org.gradle.internal.execution.steps.CreateOutputsStep.execute(CreateOutputsStep.java:29)
        at org.gradle.internal.execution.steps.CaptureStateAfterExecutionStep.executeDelegateBroadcastingChanges(CaptureStateAfterExecutionStep.java:124)
        at org.gradle.internal.execution.steps.CaptureStateAfterExecutionStep.execute(CaptureStateAfterExecutionStep.java:80)
        at org.gradle.internal.execution.steps.CaptureStateAfterExecutionStep.execute(CaptureStateAfterExecutionStep.java:58)
        at org.gradle.internal.execution.steps.ResolveInputChangesStep.execute(ResolveInputChangesStep.java:48)
        at org.gradle.internal.execution.steps.ResolveInputChangesStep.execute(ResolveInputChangesStep.java:36)
        at org.gradle.internal.execution.steps.BuildCacheStep.executeWithoutCache(BuildCacheStep.java:181)
        at org.gradle.internal.execution.steps.BuildCacheStep.executeAndStoreInCache(BuildCacheStep.java:154)
        at org.gradle.internal.execution.steps.BuildCacheStep.lambda$executeWithCache$4(BuildCacheStep.java:121)
        at org.gradle.internal.execution.steps.BuildCacheStep.lambda$executeWithCache$5(BuildCacheStep.java:121)
        at org.gradle.internal.Try$Success.map(Try.java:164)
        at org.gradle.internal.execution.steps.BuildCacheStep.executeWithCache(BuildCacheStep.java:81)
        at org.gradle.internal.execution.steps.BuildCacheStep.lambda$execute$0(BuildCacheStep.java:70)
        at org.gradle.internal.Either$Left.fold(Either.java:115)
        at org.gradle.internal.execution.caching.CachingState.fold(CachingState.java:59)
        at org.gradle.internal.execution.steps.BuildCacheStep.execute(BuildCacheStep.java:69)
        at org.gradle.internal.execution.steps.BuildCacheStep.execute(BuildCacheStep.java:47)
        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:110)
        at org.gradle.internal.execution.steps.SkipUpToDateStep.lambda$execute$2(SkipUpToDateStep.java:56)
        at org.gradle.internal.execution.steps.SkipUpToDateStep.execute(SkipUpToDateStep.java:56)
        at org.gradle.internal.execution.steps.SkipUpToDateStep.execute(SkipUpToDateStep.java:38)
        at org.gradle.internal.execution.steps.ResolveChangesStep.execute(ResolveChangesStep.java:73)
        at org.gradle.internal.execution.steps.ResolveChangesStep.execute(ResolveChangesStep.java:44)
        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:89)
        at org.gradle.internal.execution.steps.ResolveCachingStateStep.execute(ResolveCachingStateStep.java:50)
        at org.gradle.internal.execution.steps.ValidateStep.execute(ValidateStep.java:102)
        at org.gradle.internal.execution.steps.ValidateStep.execute(ValidateStep.java:57)
        at org.gradle.internal.execution.steps.CaptureStateBeforeExecutionStep.execute(CaptureStateBeforeExecutionStep.java:76)
        at org.gradle.internal.execution.steps.CaptureStateBeforeExecutionStep.execute(CaptureStateBeforeExecutionStep.java:50)
        at org.gradle.internal.execution.steps.SkipEmptyWorkStep.executeWithNoEmptySources(SkipEmptyWorkStep.java:254)
        at org.gradle.internal.execution.steps.SkipEmptyWorkStep.execute(SkipEmptyWorkStep.java:91)
        at org.gradle.internal.execution.steps.SkipEmptyWorkStep.execute(SkipEmptyWorkStep.java:56)
        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:43)
        at org.gradle.internal.execution.steps.LoadPreviousExecutionStateStep.execute(LoadPreviousExecutionStateStep.java:31)
        at org.gradle.internal.execution.steps.AssignWorkspaceStep.lambda$execute$0(AssignWorkspaceStep.java:40)
        at org.gradle.api.internal.tasks.execution.TaskExecution$4.withWorkspace(TaskExecution.java:281)
        at org.gradle.internal.execution.steps.AssignWorkspaceStep.execute(AssignWorkspaceStep.java:40)
        at org.gradle.internal.execution.steps.AssignWorkspaceStep.execute(AssignWorkspaceStep.java:30)
        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:44)
        at org.gradle.internal.execution.steps.IdentifyStep.execute(IdentifyStep.java:33)
        at org.gradle.internal.execution.impl.DefaultExecutionEngine$1.execute(DefaultExecutionEngine.java:76)
        at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.executeIfValid(ExecuteActionsTaskExecuter.java:139)
        at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.execute(ExecuteActionsTaskExecuter.java:128)
        at org.gradle.api.internal.tasks.execution.CleanupStaleOutputsExecuter.execute(CleanupStaleOutputsExecuter.java:77)
        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:57)
        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:69)
        at org.gradle.execution.taskgraph.DefaultTaskExecutionGraph$InvokeNodeExecutorsAction.execute(DefaultTaskExecutionGraph.java:322)
        at org.gradle.execution.taskgraph.DefaultTaskExecutionGraph$InvokeNodeExecutorsAction.execute(DefaultTaskExecutionGraph.java:309)
        at org.gradle.execution.taskgraph.DefaultTaskExecutionGraph$BuildOperationAwareExecutionAction.execute(DefaultTaskExecutionGraph.java:302)
        at org.gradle.execution.taskgraph.DefaultTaskExecutionGraph$BuildOperationAwareExecutionAction.execute(DefaultTaskExecutionGraph.java:288)
        at org.gradle.execution.plan.DefaultPlanExecutor$ExecutorWorker.execute(DefaultPlanExecutor.java:462)
        at org.gradle.execution.plan.DefaultPlanExecutor$ExecutorWorker.run(DefaultPlanExecutor.java:379)
        at org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:64)
        at org.gradle.internal.concurrent.ManagedExecutorImpl$1.run(ManagedExecutorImpl.java:49)```

@paul-dingemans
Copy link
Collaborator

In your case you should not suppress the semi but the trailing-comma-on-declaration-site rule. Following should work:

@Suppress("ktlint:trailing-comma-on-declaration-site")
enum class FileType(val extension: String) {
    PB("pb"),
    PBTXT("pbtxt");

    fun extensionSupported(extension: String): Boolean = this.extension == extension
}

fun File.isPb(): Boolean = FileType.PB.extensionSupported(extension.lowercase())

@paul-dingemans paul-dingemans unpinned this issue Jun 27, 2023
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

Successfully merging a pull request may close this issue.

6 participants