Kotlin / KEEP

Kotlin Evolution and Enhancement Process
Apache License 2.0
3.3k stars 356 forks source link

Explicit mode for library authoring #45

Closed ilya-g closed 3 years ago

ilya-g commented 7 years ago

Motivation

There were a couple of hot discussions about the default visibility before release: https://discuss.kotlinlang.org/t/public-by-default-for-classes/110 https://discuss.kotlinlang.org/t/kotlins-default-visibility-should-be-internal/1400

The main concern against public-by-default visibility was that it becomes too easy for library authors to expose something accidentally, release it, and then have to make breaking changes to hide it back.

The proposal

The proposal is to provide an 'Explicit API' mode in the compiler which helps library authoring. Such mode should prevent delivery of unintended public API/ABI to the clients by requiring explicit visibility modifier and explicit return types for public declarations.

See the full proposal text in https://github.com/Kotlin/KEEP/blob/master/proposals/explicit-api-mode.md

cypressious commented 7 years ago

Have you considered including open/final in the explicit mode?

ilya-g commented 7 years ago

@cypressious, Current situation with final-by-default is safe from the point of library authoring, one have to be explicit to make a class more accessible/extensible. What benefits do you expect explicit final would bring?

cypressious commented 7 years ago

The main benefit is choice. Maybe some library authors don't want final by default. See https://www.reddit.com/r/programming/comments/4yi4ch/kotlin_the_good_the_bad_and_the_ugly/ for a controversial discussion about this topic.

JakeWharton commented 7 years ago

Are those library authors? Or are they application developers complaining about libraries because they've never had to maintain one of non-trivial popularity. I'm betting the latter.

On Sat, Aug 20, 2016, 9:58 AM Kirill Rakhman notifications@github.com wrote:

The main benefit is choice. Maybe some library authors don't want final by default. See https://www.reddit.com/r/programming/comments/4yi4ch/kotlin_the_good_the_bad_and_the_ugly/ for a controversial discussion about this topic.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Kotlin/KEEP/issues/45#issuecomment-241201614, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEEEUQ-EjvRmJjQ0bLZrzz7ah_hb0U3ks5qhwfzgaJpZM4JpE0c .

ilya-g commented 7 years ago

I think there should be exceptions even in the explicit public mode, that can have public omitted and still have it by default:

What do you think?

mikehearn commented 7 years ago

It might be nice if the compiler would emit a warning in "api mode" when you have something public which doesn't have a kdoc (or doesn't inherit one)

wyand commented 7 years ago

There are good reasons mentioned here to enforce some constraints on code style -- but I think adding these directly to the compiler as options is a step in the wrong direction.

Look at any release management tooling, like checkstyle, or anything that scans public API definitions for changes, and you find a forest of options. It is a large and complex problem, and it does not have a one-size-fits-all solution. I would strongly recommend looking at external tooling to enforce code style (and expose public API changes, for the library management arguments), instead of bloating the compiler.

It would be absolutely wonderful for that external tooling to be a core part of the Kotlin ecosystem, but I don't think it belongs in the compiler.

passsy commented 7 years ago

To be clear, this proposal is about having no default visibility. This requires the author to explicitly define visibilities everywhere.

I was missing the alternatives paragraph in the proposal comparing no-default-visibility to internal-by-default

Alternatives

internal-by-default

ilya-g commented 7 years ago

@passsy Changing the default visibility is out of scope of this proposal.

cjkent commented 6 years ago

The thing that worries me about this proposal is that it would mean Kotlin is no longer a single language, but a family of related but not identical languages. I would not look forward to a future where I could take a snippet of Kotlin code and would also need to know the matching set of compiler options needed to make it compile.

I remember reading that the designers of Java explicitly decided not to allow compiler options that could affect whether a piece of code would compile or not.

I agree with @wyand that this is a problem that would be better solved using external tools like CheckStyle.

udalov commented 6 years ago

@cjkent There's nothing in this proposal that would require users to know compiler options to correctly compile the code. The proposal literally says

Explicit mode doesn't change the semantics of the correct code, so it's safe from the standpoint of compatibility. For example, if some code was compiled once in explicit mode and later without explicit mode, the result should be the same.

Which means that any correct Kotlin code can be compiled in the normal mode (without any extra flags), and the explicit library mode will not change how your program behaves, it will only cause the compiler to report a bunch of extra diagnostics.

cjkent commented 6 years ago

Fair point, any Kotlin code can be compiled in normal mode. But setting the compiler option for explicit mode would mean that some Kotlin code valid in normal mode would not compile. This doesn't seem particularly desirable.

This is effectively a code style issue. Does it really need compiler support?

mikehearn commented 6 years ago

Kotlin is already a family of not really compatible languages (/JS, /JVM and /Native) so for better or worse that bridge was already crossed.

The point of explicit mode is indeed to avoid code compiling that would be accepted in normal mode. That's the benefit of the feature. As TL of a project that produces a large API we'd definitely benefit from this. Whilst it can be seen as a code style issue, it's really more of a correctness issue. And the compiler should be helping you write correct code. This is not much different to having various -W flags in GCC. If you use -Werror, then it also means you can take arbitrary C code off the internet and find it doesn't compile. That doesn't mean C is fragmented into hundreds of sub-languages though. It just means you may need to relax checking for some kinds of code.

cjkent commented 6 years ago

I'm not sure how it is a correctness issue to omit visibility modifiers from a language that doesn't require them by default. Surely that's the very definition of a style issue?

Where do you draw the line? Should there be a compiler option that refuses to compile any code containing tabs? Come to think of it that doesn't sound like a bad idea :)

udalov commented 6 years ago

The line is very clear to me here: if you can accidentally deliver incorrect or unwanted API/ABI to your users because the compiler did not warn you that omitting something is risky, then it makes sense to prohibit that omission in the explicit library mode. In the case of missing 'public', you can accidentally expose an API that you did not intend to. And in the case of missing return types, you can accidentally change return type of an exposed function by refactoring your project internals. You can't accidentally do any harm by using tabs instead of spaces, so it's out of scope of the explicit library mode.

gildor commented 6 years ago

Any updates? Is there anything that we should discuss? I think this is a very helpful proposal for any library author. There are more and more Kotlin libraries and it would be a good help for authors and users

lppedd commented 5 years ago

Just a bump to keep this alive. It's a smart idea!

Schahen commented 4 years ago

I want to discuss this issue again in context of external declarations - for them public modifier is redundant - can we please clarify whether we still insist that we want to have public modifiers for them?

If we don't - we should explicitly mention this. If we do - again, let's discuss why exactly?

ilya-g commented 4 years ago

How are external declarations in any way special compared to the other declarations? External declarations can be private, internal, public, and the latter is currently the default and therefore redundant visibility modifier for them, but it's the same as for the other declarations.

The goal of this proposal is to make a mode where public modifier becomes required for the most of Kotlin declarations. I don't see why we should make an exception for external declarations.

shabunc commented 4 years ago
  1. Will the "redundant visibility modifier" warning removed from IDE?

    Screen Shot 2019-10-01 at 9 01 11 AM
  2. Will top level declarations without public modifier as public ones by kotlin spec? From what I've read it's supposed to be a separate flag in compiler?

shabunc commented 4 years ago

How are external declarations in any way special compared to the other declarations?

Because they by their nature in 99% percent of cases supposed to be public.

Schahen commented 4 years ago

At least I suggest us to have an file annotation that suppresses such behavior - exactly for the reasons why we do have such suppression annotations for a bunch of other cases.

joffrey-bion commented 4 years ago

How are external declarations in any way special compared to the other declarations?

Because they by their nature in 99% percent of cases supposed to be public.

@Schahen I don't really get your point here. The nature of external declarations is to make JS code available to Kotlin code.

There is actually 2 main use cases I can see so far:

  1. you declare externals in a single module to be reused in other projects, just like you would publish the typescript definitions for an NPM package. This one will mainly have everything public, yes, but I don't think this is the kind of project that would benefit from this explicit library mode, so you would probably not turn it on anyway.
  2. you write a multiplatform or JS library and use NPM dependencies with externals. In this case, you most likely don't want to expose your JS dependencies' API transitively, and will stick to internal. This is more the kind of projects impacted by the explicit library mode, and for this one, public won't be as prevalent as you seem to imply.
ilya-g commented 3 years ago

The explicit API mode is finally implemented in Kotlin 1.4.

If you have any further improvement proposals, please submit them to the Kotlin issue tracker: https://youtrack.jetbrains.com/issues/KT