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

Enable explicit API mode #3139

Merged
merged 2 commits into from
Aug 31, 2023
Merged

Enable explicit API mode #3139

merged 2 commits into from
Aug 31, 2023

Conversation

IgnatBeresnev
Copy link
Member

Depends on #3138, will rebase onto master once it gets merged.

This PR enables explicit API mode, which adds compiler checks for explicit visibility modifiers and return types.

You can see by the changes in the .api files that I changed some of the return types - I believe they were inferred incorrectly, as in most cases the supertype was deduced instead of the interface / base type. Other than that, I think everything should be fine in terms of backward compatibility.

Additionally, because visibility modifiers and return types made the lines longer, I did some small reformatting along the way, mostly adding line breaks or converting functions to block bodies.

@IgnatBeresnev IgnatBeresnev self-assigned this Aug 24, 2023
val projectsWithoutOptInDependency = setOf(
":integration-tests", ":integration-tests:gradle", ":integration-tests:maven", ":integration-tests:cli")
kotlin {
explicitApi = ExplicitApiMode.Strict
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If explicitApi was set in tasks.withType<KotlinCompile>(), it would be applied to the test sources as well, which we don't want. But setting it in kotlin {} fixes it, so I did that and moved other compiler arguments into it as well.

Comment on lines -107 to +126
fun ContentMatcherBuilder<*>.list(block: ContentMatcherBuilder<ContentList>.() -> Unit) = composite(block)
public fun ContentMatcherBuilder<*>.list(block: ContentMatcherBuilder<ContentList>.() -> Unit) {
composite(block)
}
Copy link
Member Author

@IgnatBeresnev IgnatBeresnev Aug 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of specifying the : Unit return type, I converted void functions to block bodies. Didn't think it would lead to so many changes lines, sorry if it makes it difficult to review :(

@IgnatBeresnev IgnatBeresnev force-pushed the explicit-api-mode branch 2 times, most recently from c2ffa20 to 8d7e77a Compare August 30, 2023 16:17
@IgnatBeresnev IgnatBeresnev changed the base branch from junit5 to master August 30, 2023 16:17
@IgnatBeresnev
Copy link
Member Author

Rebased the branch onto master and changed the base branch to master as well, can be reviewed safely, won't do it again

@IgnatBeresnev
Copy link
Member Author

IgnatBeresnev commented Aug 31, 2023

Had to rebase and force push again after #3149, there were too many conflicts

Language settings weren't applied to the integrationTest module before by mistake, but now they are, so assertContains is not available
@IgnatBeresnev IgnatBeresnev merged commit 02f30b1 into master Aug 31, 2023
11 checks passed
@IgnatBeresnev IgnatBeresnev deleted the explicit-api-mode branch August 31, 2023 18:16
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 this pull request may close these issues.

None yet

2 participants