pinterest / ktlint

An anti-bikeshedding Kotlin linter with built-in formatter
https://pinterest.github.io/ktlint/
MIT License
6.23k stars 512 forks source link

Ignore comments in imports when formatting #2670

Closed Sti2nd closed 5 months ago

Sti2nd commented 5 months ago

Expected Behavior

I expect Ktlint to proceeed (as opposed to stop and log error) when there are comments in the code. As an anti-bikeshedding tool it is weird that Ktlint wants us to move comments before sorting imports (i.e. bike-shedding activities). Other tools (like IntelliJ) do not stop when one run format or organize imports with comments in them.

Current Behavior

Ktlint logs the following error:

Imports must be ordered according to the pattern specified in .editorconfig -- no autocorrection due to comments in the import list (cannot be auto-corrected)

Additional information

Ktlint already recognises comments, so why not ignore them insted of stopping and logging.

id("org.jlleitschuh.gradle.ktlint") version "12.0.3"

.editorconfig

```text root = true [*] end_of_line = lf charset = utf-8 indent_size = 2 indent_style = space tab_width = 2 ij_continuation_indent_size = 2 ij_formatter_off_tag = @formatter:off ij_formatter_on_tag = @formatter:on ij_formatter_tags_enabled = true ij_smart_tabs = false ij_visual_guides = none ij_wrap_on_typing = false ij_properties_keep_blank_lines = true [*.sql] tab_width = 4 indent_size = 4 [{*.htm,*.html,*.ng,*.sht,*.shtm,*.shtml}] ij_html_tab_width = 2 ij_html_indent_size = 2 ij_html_continuation_indent_size = 2 ij_html_align_attributes = false ij_html_space_inside_empty_tag = false [*.css] indent_size = 4 tab_width = 4 ij_continuation_indent_size = 4 ij_css_continuation_indent_size = 4 [.editorconfig] ij_editorconfig_space_after_comma = false ij_editorconfig_space_before_comma = false [{*.yml,*.yaml}] indent_size = 4 ij_continuation_indent_size = 4 ij_yaml_spaces_within_braces = false ij_yaml_spaces_within_brackets = false [*.json] ij_json_spaces_within_braces = false ij_json_spaces_within_brackets = false [*.jsonl] ij_formatter_enabled = false [{*.kt,*.kts,*.java}] indent_size = 4 ij_continuation_indent_size = 4 trim_trailing_whitespace = true max_line_length = 120 [*.java] ij_java_imports_layout = |,$com.**,$java.**,$javax.**,$org.**,$*,|,java.**,|,javax.**,|,org.**,|,com.**,|,* ij_java_class_count_to_use_import_on_demand = 99 ij_java_names_count_to_use_import_on_demand = 99 ij_java_keep_control_statement_in_one_line = false ij_java_keep_blank_lines_in_declarations = 1 ij_any_keep_blank_lines_before_right_brace = 1 ij_java_indent_case_from_switch = false ij_java_align_multiline_parameters = false ij_java_spaces_within_braces = true ij_java_spaces_within_array_initializer_braces = true ij_java_space_before_array_initializer_left_brace = true ij_java_call_parameters_wrap = normal ij_java_resource_list_wrap = normal ij_java_extends_list_wrap = normal ij_java_throws_list_wrap = normal ij_java_extends_keyword_wrap = normal ij_java_method_call_chain_wrap = normal ij_java_binary_operation_wrap = normal ij_java_binary_operation_sign_on_next_line = true ij_java_ternary_operation_wrap = on_every_item ij_java_ternary_operation_signs_on_next_line = true ij_java_keep_simple_methods_in_one_line = true ij_java_keep_simple_classes_in_one_line = true ij_java_array_initializer_wrap = normal ij_java_assignment_wrap = normal ij_java_assert_statement_wrap = normal ij_java_if_brace_force = always ij_java_do_while_brace_force = always ij_java_wrap_long_lines = true ij_java_parameter_annotation_wrap = normal ij_java_variable_annotation_wrap = split_into_lines ij_java_enum_constants_wrap = split_into_lines [{*.kt,*.kts}] ij_kotlin_code_style_defaults = KOTLIN_OFFICIAL ij_kotlin_imports_layout = java.**,javax.**,kotlin.**,org.**,com.**,*,^ ij_kotlin_packages_to_use_import_on_demand = 99 ij_kotlin_name_count_to_use_star_import = 2147483647 ij_kotlin_name_count_to_use_star_import_for_members = 2147483647 ij_kotlin_keep_blank_lines_in_declarations = 1 ij_kotlin_keep_blank_lines_in_code = 1 ij_kotlin_keep_blank_lines_before_right_brace = 0 ij_kotlin_align_multiline_parameters = false ij_kotlin_line_comment_at_first_column = false ij_kotlin_line_comment_add_space = true ktlint_ignore_back_ticked_identifier = true ```
paul-dingemans commented 5 months ago

Please add a code sample, and .editorconfig, and the output you get. I expect that with 'throwing an error' you do not mean that an exception is thrown (if so, include the stacktrace).

Sti2nd commented 5 months ago

Thank you for the reply. Could be that Ktlint is just logging with error level so I rephrased. Also added the .editorconfig file (in details)

paul-dingemans commented 5 months ago

You have still not supplied your code sample. Of course I can add any random comment in between, but I would like to understand your use case to use comments in the import list.

But anyways, comments in the import list can not just be ignored. From the parsers perspective comments are not bound with a specific import. So when reordering the imports, assumptions have to be made to which import a comment belongs. It is too simple to assume that a comment is always associated with the next import:

// comment before foo
import foo
// comment after foo
// comment before bar

import bar

From the perspective of the parser, it is unclear that comment after foo should be kept close to the import foo while comment before bar (and the blank line) have to be kept together with import bar.

Sti2nd commented 5 months ago

Thank you for asking for the use case! The use case is that we use ktlint for auto-formatting. I am used to Prettier and CSharpier which are just formatters, so I have actually forgotten ktlint is more than a formatter.

We run ktlint on compile (and git commit). I believe these are the relevant gradle tasks

tasks.withType<KotlinCompile> {
    dependsOn("addPreCommitHook")
    dependsOn("ktlintFormat")
    kotlinOptions {
        freeCompilerArgs = listOf("-Xjsr305=strict")
        jvmTarget = "21"
    }
}

tasks.withType<KtLintCheckTask> {
    dependsOn("ktlintFormat")
}

Running a test requires compilation first, which requires ktlint to be happy. Ktlint will complain about unused imports, so then I figured commenting out the unused imports could make ktlint happy. Code example:

// import org.springframework.http.ProblemDetail Uncommented because it is unused right now and I want to run a test
import org.springframework.http.ResponseEntity
import org.springframework.validation.annotation.Validated
import org.springframework.web.bind.annotation.RequestBody
import org.springframework.web.bind.annotation.RequestMapping
import org.springframework.web.bind.annotation.RequestMethod
import org.springframework.web.bind.annotation.RestController
import io.swagger.v3.oas.annotations.Operation
import io.swagger.v3.oas.annotations.Parameter
import io.swagger.v3.oas.annotations.media.Content
import io.swagger.v3.oas.annotations.media.Schema
import io.swagger.v3.oas.annotations.responses.ApiResponse
import jakarta.validation.Valid
paul-dingemans commented 5 months ago

You might consider to add suppression below (must be before the package statement):

@file:Suppress("ktlint:standard:no-unused-imports")

In this way you do not need to (de)comment the import. Downside is that unused imports in this particular file will not be reported anymore.

Sti2nd commented 5 months ago

Hm. I think you are right. If we just want formatting, removing unused imports are out of scope (i.e. linting). Do you know of an easy way to disable all rules that cannot be auto-fixed? Or can Ktlint run in "formatting mode" and disable linting?

paul-dingemans commented 5 months ago

A rule can emit both violations that can be autocorrected as well as errors that can not be autocorrected. So there is no easy fix to disable rules that can not be autocorrected.

But it looks that you just want to run format, and ignore it's return code. All violations with an autocorrect will be fixed. At least this works with the ktlint CLI. I don't know how the ktlint gradle plugin handles this.

Sti2nd commented 5 months ago

Good thinking! Indeed I am just interested in the "side-effect" that is formatting. I will look into how I can ignore errors in gradle. Will try to remember to update this thread if I find something useful. And thank you for the good help :)