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

Support factory methods using generics (ktlint-ruleset-standard) #2350

Closed
sya-ri opened this issue Nov 13, 2023 · 3 comments · Fixed by #2366
Closed

Support factory methods using generics (ktlint-ruleset-standard) #2350

sya-ri opened this issue Nov 13, 2023 · 3 comments · Fixed by #2366
Milestone

Comments

@sya-ri
Copy link
Contributor

sya-ri commented Nov 13, 2023

Expected Behavior

class Generics<T>(val value: T)

// Error
fun <T> Generics(action: () -> T): Generics<T> = Generics(action())

Expect this code to not error.

ASTNode.isFactoryMethod() needs to be improved.

it.name == it.typeReference?.text

it.name is a method name, and it.typeReference?.text includes generics, so there will be a mismatch.


As a problem related to factory methods, even one-liners that use the expression body will result in an error. Since this is an avoidable problem and difficult to analyze, I don't think it's necessary to deal with it.

class NoGenerics(val value: String)

// Error
fun NoGenerics(value: Double) = NoGenerics(value.toString())

Observed Behavior

15:28:16.167 [main] DEBUG com.pinterest.ktlint.cli.internal.KtlintServiceLoader - Discovered RuleSetProviderV3 with id 'standard' in ktlint JAR
15:28:16.392 [main] DEBUG com.pinterest.ktlint.cli.internal.KtlintServiceLoader - Discovered ReporterProviderV2 with id 'baseline' in ktlint JAR
15:28:16.392 [main] DEBUG com.pinterest.ktlint.cli.internal.KtlintServiceLoader - Discovered ReporterProviderV2 with id 'plain' in ktlint JAR
15:28:16.392 [main] DEBUG com.pinterest.ktlint.cli.internal.KtlintServiceLoader - Discovered ReporterProviderV2 with id 'checkstyle' in ktlint JAR
15:28:16.392 [main] DEBUG com.pinterest.ktlint.cli.internal.KtlintServiceLoader - Discovered ReporterProviderV2 with id 'json' in ktlint JAR
15:28:16.392 [main] DEBUG com.pinterest.ktlint.cli.internal.KtlintServiceLoader - Discovered ReporterProviderV2 with id 'format' in ktlint JAR
15:28:16.392 [main] DEBUG com.pinterest.ktlint.cli.internal.KtlintServiceLoader - Discovered ReporterProviderV2 with id 'html' in ktlint JAR
15:28:16.392 [main] DEBUG com.pinterest.ktlint.cli.internal.KtlintServiceLoader - Discovered ReporterProviderV2 with id 'plain-summary' in ktlint JAR
15:28:16.392 [main] DEBUG com.pinterest.ktlint.cli.internal.KtlintServiceLoader - Discovered ReporterProviderV2 with id 'sarif' in ktlint JAR
15:28:16.393 [main] DEBUG com.pinterest.ktlint.cli.internal.ReporterAggregator - Initializing "plain" reporter with {plain=true, color=false, color_name=DARK_GRAY, format=false}
15:28:16.398 [main] DEBUG com.pinterest.ktlint.cli.internal.FileUtils - Start walkFileTree from directory: 'C:\Users\gbv_s\IdeaProjects\ktlint-function-naming-factory-method-bug'
15:28:16.411 [main] DEBUG com.pinterest.ktlint.cli.internal.FileUtils - Discovered 3 files to be processed in 12 ms
15:28:16.429 [pool-1-thread-2] DEBUG com.pinterest.ktlint.rule.engine.api.KtLintRuleEngine - Starting with linting file 'settings.gradle.kts'
15:28:16.429 [pool-1-thread-1] DEBUG com.pinterest.ktlint.rule.engine.api.KtLintRuleEngine - Starting with linting file 'build.gradle.kts'
15:28:16.429 [pool-1-thread-3] DEBUG com.pinterest.ktlint.rule.engine.api.KtLintRuleEngine - Starting with linting file 'Main.kt'
15:28:16.790 [pool-1-thread-3] DEBUG com.pinterest.ktlint.rule.engine.internal.EditorConfigLoader - Effective editorconfig properties for file 'C:\Users\gbv_s\IdeaProjects\ktlint-function-naming-factory-method-bug\src\main\kotlin\Main.kt':
	
15:28:16.790 [pool-1-thread-1] DEBUG com.pinterest.ktlint.rule.engine.internal.EditorConfigLoader - Effective editorconfig properties for file 'C:\Users\gbv_s\IdeaProjects\ktlint-function-naming-factory-method-bug\build.gradle.kts':
	
15:28:16.790 [pool-1-thread-2] DEBUG com.pinterest.ktlint.rule.engine.internal.EditorConfigLoader - Effective editorconfig properties for file 'C:\Users\gbv_s\IdeaProjects\ktlint-function-naming-factory-method-bug\settings.gradle.kts':
	
15:28:16.803 [pool-1-thread-1] DEBUG com.pinterest.ktlint.rule.engine.internal.rulefilter.RunAfterRuleFilter - Rule with id 'standard:argument-list-wrapping' should run after the rule with id 'standard:class-signature'. However, the latter rule is not loaded and is allowed to be ignored. For best results, it is advised load the rule.
15:28:16.803 [pool-1-thread-2] DEBUG com.pinterest.ktlint.rule.engine.internal.rulefilter.RunAfterRuleFilter - Rule with id 'standard:argument-list-wrapping' should run after the rule with id 'standard:class-signature'. However, the latter rule is not loaded and is allowed to be ignored. For best results, it is advised load the rule.
15:28:16.803 [pool-1-thread-1] DEBUG com.pinterest.ktlint.rule.engine.internal.rulefilter.RunAfterRuleFilter - Rule with id 'standard:indent' should run after the rule with id 'standard:class-signature'. However, the latter rule is not loaded and is allowed to be ignored. For best results, it is advised load the rule.
15:28:16.803 [pool-1-thread-2] DEBUG com.pinterest.ktlint.rule.engine.internal.rulefilter.RunAfterRuleFilter - Rule with id 'standard:indent' should run after the rule with id 'standard:class-signature'. However, the latter rule is not loaded and is allowed to be ignored. For best results, it is advised load the rule.
15:28:16.803 [pool-1-thread-3] DEBUG com.pinterest.ktlint.rule.engine.internal.rulefilter.RunAfterRuleFilter - Rule with id 'standard:argument-list-wrapping' should run after the rule with id 'standard:class-signature'. However, the latter rule is not loaded and is allowed to be ignored. For best results, it is advised load the rule.
15:28:16.804 [pool-1-thread-3] DEBUG com.pinterest.ktlint.rule.engine.internal.rulefilter.RunAfterRuleFilter - Rule with id 'standard:indent' should run after the rule with id 'standard:class-signature'. However, the latter rule is not loaded and is allowed to be ignored. For best results, it is advised load the rule.
15:28:16.804 [pool-1-thread-2] DEBUG com.pinterest.ktlint.rule.engine.internal.rulefilter.RunAfterRuleFilter - Rule with id 'standard:argument-list-wrapping' should run after the rule with id 'standard:class-signature'. However, the latter rule is not loaded and is allowed to be ignored. For best results, it is advised load the rule.
15:28:16.804 [pool-1-thread-1] DEBUG com.pinterest.ktlint.rule.engine.internal.rulefilter.RunAfterRuleFilter - Rule with id 'standard:argument-list-wrapping' should run after the rule with id 'standard:class-signature'. However, the latter rule is not loaded and is allowed to be ignored. For best results, it is advised load the rule.
15:28:16.804 [pool-1-thread-2] DEBUG com.pinterest.ktlint.rule.engine.internal.rulefilter.RunAfterRuleFilter - Rule with id 'standard:indent' should run after the rule with id 'standard:class-signature'. However, the latter rule is not loaded and is allowed to be ignored. For best results, it is advised load the rule.
15:28:16.804 [pool-1-thread-1] DEBUG com.pinterest.ktlint.rule.engine.internal.rulefilter.RunAfterRuleFilter - Rule with id 'standard:indent' should run after the rule with id 'standard:class-signature'. However, the latter rule is not loaded and is allowed to be ignored. For best results, it is advised load the rule.
15:28:16.804 [pool-1-thread-3] DEBUG com.pinterest.ktlint.rule.engine.internal.rulefilter.RunAfterRuleFilter - Rule with id 'standard:argument-list-wrapping' should run after the rule with id 'standard:class-signature'. However, the latter rule is not loaded and is allowed to be ignored. For best results, it is advised load the rule.
15:28:16.804 [pool-1-thread-3] DEBUG com.pinterest.ktlint.rule.engine.internal.rulefilter.RunAfterRuleFilter - Rule with id 'standard:indent' should run after the rule with id 'standard:class-signature'. However, the latter rule is not loaded and is allowed to be ignored. For best results, it is advised load the rule.
15:28:16.804 [pool-1-thread-2] DEBUG com.pinterest.ktlint.rule.engine.internal.rulefilter.RunAfterRuleFilter - Rule with id 'standard:indent' should run after the rule with id 'standard:class-signature'. However, the latter rule is not loaded and is allowed to be ignored. For best results, it is advised load the rule.
15:28:16.805 [pool-1-thread-1] DEBUG com.pinterest.ktlint.rule.engine.internal.rulefilter.RunAfterRuleFilter - Rule with id 'standard:indent' should run after the rule with id 'standard:class-signature'. However, the latter rule is not loaded and is allowed to be ignored. For best results, it is advised load the rule.
15:28:16.805 [pool-1-thread-3] DEBUG com.pinterest.ktlint.rule.engine.internal.rulefilter.RunAfterRuleFilter - Rule with id 'standard:indent' should run after the rule with id 'standard:class-signature'. However, the latter rule is not loaded and is allowed to be ignored. For best results, it is advised load the rule.
15:28:16.815 [pool-1-thread-2] DEBUG com.pinterest.ktlint.rule.engine.internal.RuleProviderSorter - Rules will be executed in order below:
           - internal:ktlint-suppression, 
           - standard:annotation-spacing, 
           - standard:blank-line-before-declaration, 
           - standard:chain-wrapping, 
           - standard:class-naming, 
           - standard:colon-spacing, 
           - standard:comma-spacing, 
           - standard:comment-spacing, 
           - standard:comment-wrapping, 
           - standard:context-receiver-wrapping, 
           - standard:curly-spacing, 
           - standard:discouraged-comment-location, 
           - standard:dot-spacing, 
           - standard:double-colon-spacing, 
           - standard:enum-entry-name-case, 
           - standard:enum-wrapping, 
           - standard:filename, 
           - standard:final-newline, 
           - standard:fun-keyword-spacing, 
           - standard:function-naming, 
           - standard:function-return-type-spacing, 
           - standard:function-start-of-body-spacing, 
           - standard:function-type-reference-spacing, 
           - standard:if-else-bracing, 
           - standard:import-ordering, 
           - standard:kdoc-wrapping, 
           - standard:keyword-spacing, 
           - standard:modifier-list-spacing, 
           - standard:modifier-order, 
           - standard:multiline-expression-wrapping, 
           - standard:multiline-if-else, 
           - standard:no-blank-line-before-rbrace, 
           - standard:no-blank-line-in-list, 
           - standard:no-blank-lines-in-chained-method-calls, 
           - standard:no-consecutive-blank-lines, 
           - standard:no-consecutive-comments, 
           - standard:no-empty-class-body, 
           - standard:no-empty-file, 
           - standard:no-empty-first-line-in-class-body, 
           - standard:no-empty-first-line-in-method-block, 
           - standard:no-line-break-after-else, 
           - standard:no-line-break-before-assignment, 
           - standard:no-multi-spaces, 
           - standard:no-trailing-spaces, 
           - standard:no-unit-return, 
           - standard:no-unused-imports, 
           - standard:no-wildcard-imports, 
           - standard:nullable-type-spacing, 
           - standard:op-spacing, 
           - standard:package-name, 
           - standard:parameter-list-spacing, 
           - standard:parameter-list-wrapping, 
           - standard:parameter-wrapping, 
           - standard:paren-spacing, 
           - standard:property-naming, 
           - standard:property-wrapping, 
           - standard:range-spacing, 
           - standard:spacing-around-angle-brackets, 
           - standard:spacing-between-declarations-with-annotations, 
           - standard:spacing-between-declarations-with-comments, 
           - standard:spacing-between-function-name-and-opening-parenthesis, 
           - standard:statement-wrapping, 
           - standard:string-template, 
           - standard:try-catch-finally-spacing, 
           - standard:type-argument-list-spacing, 
           - standard:type-parameter-list-spacing, 
           - standard:unary-op-spacing, 
           - standard:unnecessary-parentheses-before-trailing-lambda, 
           - standard:annotation, 
           - standard:if-else-wrapping, 
           - standard:no-single-line-block-comment, 
           - standard:wrapping, 
           - standard:argument-list-wrapping, 
           - standard:no-semi, 
           - standard:function-signature, 
           - standard:trailing-comma-on-call-site, 
           - standard:trailing-comma-on-declaration-site, 
           - standard:indent, 
           - standard:block-comment-initial-star-alignment, 
           - standard:string-template-indent, 
           - standard:max-line-length
15:28:16.990 [pool-1-thread-2] DEBUG com.pinterest.ktlint.rule.engine.api.KtLintRuleEngine - Finished with linting file 'settings.gradle.kts'
15:28:16.991 [pool-1-thread-1] DEBUG com.pinterest.ktlint.rule.engine.api.KtLintRuleEngine - Finished with linting file 'build.gradle.kts'
15:28:17.017 [pool-1-thread-3] DEBUG com.pinterest.ktlint.rule.engine.api.KtLintRuleEngine - Finished with linting file 'Main.kt'
C:/Users/gbv_s/IdeaProjects/ktlint-function-naming-factory-method-bug/src/main/kotlin/Main.kt:12:5: Function name should start with a lowercase letter (except factory methods) and use camel case (standard:function-naming)
C:/Users/gbv_s/IdeaProjects/ktlint-function-naming-factory-method-bug/src/main/kotlin/Main.kt:15:9: Function name should start with a lowercase letter (except factory methods) and use camel case (standard:function-naming)

Summary error count (descending) by rule:
  standard:function-naming: 2
15:28:17.019 [main] DEBUG com.pinterest.ktlint.cli.internal.KtlintCommandLine - Finished processing in 864ms / 3 file(s) scanned / 2 error(s) found
15:28:17.019 [main] DEBUG com.pinterest.ktlint.cli.internal.KtlintCommandLine - Exit ktlint with exit code: 1

Steps to Reproduce

https://github.com/sya-ri/ktlint-function-naming-factory-method-bug

class NoGenerics(val value: String)

class Generics<T>(val value: T)

// No error
fun NoGenerics(value: Int): NoGenerics = NoGenerics(value.toString())

// No error
fun <T> NoGenerics(value: T): NoGenerics = NoGenerics(value.toString())

// Error: support one-liner
fun NoGenerics(value: Double) = NoGenerics(value.toString())

// Error
fun <T> Generics(action: () -> T): Generics<T> = Generics(action())

Your Environment

  • Version of ktlint used: 1.1.0-kotlin-dev-SNAPSHOT (ktlint-cli-1.1.0-kotlin-dev-20231106.165144-8.jar)
  • Relevant parts of the .editorconfig settings
  • Name and version (or code for custom task) of integration used (Gradle plugin, Maven plugin, command line, custom Gradle task): custom Gradle task
  • Version of Gradle used (if applicable): 8.2
  • Operating System and version: Windows 11 Home / 23H2 / 22631.2506
@sya-ri sya-ri changed the title Support factory methods using generics Support factory methods using generics (ktlint-ruleset-standard) Nov 13, 2023
@paul-dingemans
Copy link
Collaborator

Thnx for reporting and examples. I will investigate.

@paul-dingemans paul-dingemans added this to the 1.1 milestone Nov 13, 2023
paul-dingemans added a commit that referenced this issue Nov 19, 2023
…method or class constructor without specifying type

Closes #2350
@paul-dingemans
Copy link
Collaborator

// Error: support one-liner
fun NoGenerics(value: Double) = NoGenerics(value.toString())

I do see value in supporting this. I have added a kind of 'lame' implementation for this which allows exactly this example. So in this way an overload of a class constructor or another factory method can be made without having to specify the type explitcitly.

paul-dingemans added a commit that referenced this issue Nov 19, 2023
…method or class constructor without specifying type (#2366)

Closes #2350
paul-dingemans added a commit that referenced this issue Feb 13, 2024
Also, the `indent` rule should not change unexpected indentation characters inside a string template but leave this up to the `string-template-indent` rule. As a result the `indent` rule could be a bit simplified.

Closes #2350
paul-dingemans added a commit that referenced this issue Feb 13, 2024
Also, the `indent` rule should not change unexpected indentation characters inside a string template but leave this up to the `string-template-indent` rule. As a result the `indent` rule could be a bit simplified.

Closes #2350
paul-dingemans added a commit that referenced this issue Feb 14, 2024
Also, the `indent` rule should not change unexpected indentation characters inside a string template but leave this up to the `string-template-indent` rule. As a result the `indent` rule could be a bit simplified.

Closes #2350
@paul-dingemans
Copy link
Collaborator

Reference from PR #2553 is to be ignored for this issue.

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.

2 participants