pinterest / ktlint

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

`ktlint_function_signature_rule_force_multiline_when_parameter_count_greater_or_equal_than` values can't be correctly read #2560

Closed Goooler closed 4 months ago

Goooler commented 4 months ago

Expected Behavior

No exceptions were thrown.

Observed Behavior

Add

ktlint_function_signature_rule_force_multiline_when_parameter_count_greater_or_equal_than = 2

into editorconfig, and run Ktlint check in Spotless, an exception was thrown like:

Exception in thread "main" java.lang.ClassCastException: class java.lang.String cannot be cast to class java.lang.Integer (java.lang.String and java.lang.Integer are in module java.base of loader 'bootstrap')
  at com.pinterest.ktlint.ruleset.standard.rules.FunctionSignatureRule$Companion$FORCE_MULTILINE_WHEN_PARAMETER_COUNT_GREATER_OR_EQUAL_THAN_PROPERTY$1.invoke(FunctionSignatureRule.kt:761)
  at com.pinterest.ktlint.ruleset.standard.rules.FunctionSignatureRule$Companion$FORCE_MULTILINE_WHEN_PARAMETER_COUNT_GREATER_OR_EQUAL_THAN_PROPERTY$1.invoke(FunctionSignatureRule.kt:757)
  at com.pinterest.ktlint.rule.engine.core.api.editorconfig.EditorConfig.get(EditorConfig.kt:60)
  at com.pinterest.ktlint.ruleset.standard.rules.FunctionSignatureRule.beforeFirstNode(FunctionSignatureRule.kt:96)
  at com.pinterest.ktlint.rule.engine.internal.RuleExecutionContext.executeRule(RuleExecutionContext.kt:54)
  at com.pinterest.ktlint.rule.engine.api.KtLintRuleEngine$format$3.invoke(KtLintRuleEngine.kt:146)
  at com.pinterest.ktlint.rule.engine.api.KtLintRuleEngine$format$3.invoke(KtLintRuleEngine.kt:145)
  at com.pinterest.ktlint.rule.engine.internal.VisitorProvider$visitor$3.invoke(VisitorProvider.kt:46)
  at com.pinterest.ktlint.rule.engine.internal.VisitorProvider$visitor$3.invoke(VisitorProvider.kt:44)
  at com.pinterest.ktlint.rule.engine.api.KtLintRuleEngine.format(KtLintRuleEngine.kt:145)
  at com.pinterest.ktlint.rule.engine.api.KtLintRuleEngine.format$default(KtLintRuleEngine.kt:127)

This issue was originally reported to https://github.com/diffplug/spotless/issues/1904.

Steps to Reproduce

  1. Clone https://github.com/pawelkw/spotless-bug.
  2. Run ./gradlew spotlessApply

Note: I can repro this in the project above or just running

  KtLintRuleEngine(
    allRuleProviders,
    EditorConfigDefaults.load(
      editorConfigPath,
      emptySet(),
    ),
    EditorConfigOverride.EMPTY_EDITOR_CONFIG_OVERRIDE,
    false,
    editorConfigPath.fileSystem,
  ).format(Code.Companion.fromPath(mainPath))

But can't repro it via CLI running.

Your Environment

paul-dingemans commented 4 months ago

Assuming that Spotless calls KtlintRuleEngine similar to

  KtLintRuleEngine(
    allRuleProviders,
    EditorConfigDefaults.load(
      editorConfigPath,
      emptySet(),
    ),
    EditorConfigOverride.EMPTY_EDITOR_CONFIG_OVERRIDE,
    false,
    editorConfigPath.fileSystem,
  ).format(Code.Companion.fromPath(mainPath))

then the problem is caused by an incorrect call to EditorConfigDefaults.load(..). More specifically the emptySet() is causing the problem.

If code above is changed to:

  KtLintRuleEngine(
    allRuleProviders,
    EditorConfigDefaults.load(
      editorConfigPath,
      setOf(FORCE_MULTILINE_WHEN_PARAMETER_COUNT_GREATER_OR_EQUAL_THAN_PROPERTY.type),
    ),
    EditorConfigOverride.EMPTY_EDITOR_CONFIG_OVERRIDE,
    false,
    editorConfigPath.fileSystem,
  ).format(Code.Companion.fromPath(mainPath))

it will work.

Please note that this behavior is documented in the Kdoc on the function:

        /**
         * Loads properties from [path]. [path] may either locate a file (also allows specifying a file with a name other than
         * ".editorconfig") or a directory in which a file with name ".editorconfig" is expected to exist. Properties from all globs are
         * returned. If [path] is not valid then the [EMPTY_EDITOR_CONFIG_DEFAULTS] is returned.
         *
         * Properties having a custom [PropertyType] (e.g. a type not defined in the ec4j library) can not be parsed to the correct type
         * when the property type is missing and will by default be converted to type [String]. [PropertyType]'s can be easily extracted
         * with extension function `Collection<RuleProvider>.propertyTypes()`.
         *
         * The property "root" which denotes whether the parent directory is to be checked for the existence of a fallback
         * ".editorconfig" is ignored entirely.
         */
        public fun load(
            path: Path?,
            propertyTypes: Set<PropertyType<*>>,
        ): EditorConfigDefaults =

As it would be inconvenient for the API Consumer to provide all property types, ktlint has provided a helper function Collection<RuleProvider>.propertyTypes(). So changing your code as follows, ensure that similar problems for other property types won't occur:

  KtLintRuleEngine(
    allRuleProviders,
    EditorConfigDefaults.load(
      editorConfigPath,
      allRuleProviders.propertyTypes(),
    ),
    EditorConfigOverride.EMPTY_EDITOR_CONFIG_OVERRIDE,
    false,
    editorConfigPath.fileSystem,
  ).format(Code.Companion.fromPath(mainPath))
Goooler commented 4 months ago

Thanks for your explanation! I'm addressing the fix to https://github.com/diffplug/spotless/pull/2052, it would be great if you can take a review.