square / kotlinpoet

A Kotlin API for generating .kt source files.
https://square.github.io/kotlinpoet/
Apache License 2.0
3.92k stars 289 forks source link

Allow a non explicit API output mode for code that is not intending to be as formal. #1703

Closed apatrida closed 1 year ago

apatrida commented 1 year ago

I propose an output mode for ~"readability"~ non-explicit API code that can be set on the file level that differs from the current "explicit API" output mode by:

There were prior discussions about inferred types for properties, but other use cases were not mentioned nor shown (i.e. readability from the viewpoint of the framework or code generator).

See: #858 and #1014

As an example use case:

In Jooq, we might generate code that is heavily used by the user not just as an invisible generated class but also as something they refer to frequently. Therefore the source must be highly readable. Other code generated might be more invisible and can be output in the normal "strict API" mode that Kotlin Poet currently uses. Therefore some files should be output favoring readability, and others the explicit API support.

An example is a generated table class, which has fields like:

val id = column<Long>("ID") of BIGINT().identity(true)

But are output instead as:

public val id: TableField<PersonsRecord, Long> = column<Long>("ID") of BIGINT().identity(true)

This creates a lot of noise, and lowers the clarity of the desired output, such as:

    val ID = column<Long>("id") of BIGINT().identity(true)
    val NAME = column<String>("name") of VARCHAR(100)
    val BIRTHDATE = column<LocalDate?>("birthdate") of LOCALDATE()
    val STATUS = column<PersonsStatus>("status") of ENUM() 
    val BLURB = column<String?>("blurb") of VARCHAR(240)
    val PROFILE_IMAGE = column<String?>("profile_image") of VARCHAR(128)
    val PRIMARY_EMAIL = column<String?>("primary_email") of VARCHAR(128)
    val LOCATION = column<String?>("location") of VARCHAR(80)

    val IS_DELETED = column<Byte>("is_deleted") of TINYINT().defaultValue(0)
    val CREATED_AT = column<Timestamp>("created_at") of TIMESTAMP().defaultValue(DSL.currentTimestamp())
    val UPDATED_AT = column<Timestamp>("updated_at") of TIMESTAMP().defaultValue(DSL.currentTimestamp())
        .generatedAlwaysAs(DSL.currentTimestamp())
        .generationLocation(QOM.GenerationLocation.SERVER).stored()
    val DELETED_AT = column<Timestamp?>("deleted_at") of TIMESTAMP()

In the API we should be able to mark a property spec as inferable() while still providing the expected type, and depending on the output mode combined with this flag, would change between an explicit type and inferred type.

For example, this has the type specified, yet is still marked inferable for non-explicit API output mode. This could also be used in cases where the type is really difficult to specify therefore a placeholder type could be set (i.e. TableField<*,*>) knowing the type will be inferred by the generated code:

val prop = PropertySpec.builder(normalizePropertyName(column.name),
                org.jooq.TableField::class.parameterizedBy(recordClass, columnPropType))
                .initializer(CodeBlock.of(...))
                .inferable() // mark it to allow inferred types
                .build()

...

FileSpec.builder(...)
            .isExplicitApi(false) // or outout mode, you choose!
            .addType(...)
            .build()                

When creating the file spec, the non-explicit API mode vs. strict excplicit API mode is selected and the output changes. The rest of the API has no changes other than flagging to allow inferred properties.

ZacSweers commented 1 year ago

Your definition of "readability" is really subjective and never really going to be something anyone would agree on. Explicitly types and explicit modifiers aren't readability issues IMO. Formatting can be an issue, but for that you can just post process with your own formatted if you really want. Kotlinpoet cares far more about being correct.

apatrida commented 1 year ago

The name can change if "readability" is the hangup. Call it "not explicit API mode" then.

"Correct" seems subjective here too. If you say "we can only do explicit API mode" then you are "correct" ... But do all users want only explicit API mode? If not, the definition of correct becomes "what will the compiler compile" and this now includes inferred types for properties, omitting default modifiers, and other little differences.

If you will never ever, ever, ever change away from explicit API mode, then you can stand on your definition of "correct". But is there a real reason to only ever do that one mode?

apatrida commented 1 year ago

update: removed "readability" in description and went for "non explicit API mode" since that is the contention.

apatrida commented 1 year ago

you can just post process with your own formatted

Are you aware of a formatter that changes specific to inferred types other than refactorings in some IDE's?

JakeWharton commented 1 year ago

Plus you argue that our specificity is noise and harms clarity when in actuality it very objectively improves both readability and clarity. Your description of what you want and the terms you use are at odds. I think you are describing generating code that favors terseness and looks human-written and neither of those are goals of this project.

apatrida commented 1 year ago

great, glad I posted this before wasting time on a PR.

JakeWharton commented 1 year ago

But is there a real reason to only ever do that one mode?

The code generator is almost never aware of whether or not explicit API mode is enabled in the Kotlin compiler that will eventually compile the output. So if KotlinPoet exposed a setting for this then every code generator would have to either propagate that outward to their callers, or figure out how to wire it into the build (assuming they participate in the actual build). It's far easier to always comply with it since we have the information.