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

Add matchingRegex #598

Merged
merged 3 commits into from
Nov 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 2 additions & 6 deletions core/src/main/kotlin/DokkaBootstrapImpl.kt
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,7 @@ import java.util.function.BiConsumer


fun parsePerPackageOptions(args: List<String>): List<PackageOptions> = args.map { it.split(",") }.map {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this check is now unnecessary as we dropped global settings and I see no reason why the regex could not be an all match

val prefix = it.first()
if (prefix == "")
throw IllegalArgumentException(
"Please do not register packageOptions with all match pattern, use global settings instead"
)
val matchingRegex = it.first()

val args = it.subList(1, it.size)

Expand All @@ -28,7 +24,7 @@ fun parsePerPackageOptions(args: List<String>): List<PackageOptions> = args.map
?: DokkaDefaults.suppress

PackageOptionsImpl(
prefix,
matchingRegex,
includeNonPublic = privateApi,
reportUndocumented = reportUndocumented,
skipDeprecated = !deprecated,
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/kotlin/configuration.kt
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ interface DokkaConfiguration : Serializable {
}

interface PackageOptions : Serializable {
val prefix: String
val matchingRegex: String
val includeNonPublic: Boolean
val reportUndocumented: Boolean?
val skipDeprecated: Boolean
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/kotlin/defaultConfiguration.kt
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ data class SourceLinkDefinitionImpl(
}

data class PackageOptionsImpl(
override val prefix: String,
override val matchingRegex: String,
override val includeNonPublic: Boolean,
override val reportUndocumented: Boolean?,
override val skipDeprecated: Boolean,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ fun PreMergeDocumentableTransformer.sourceSet(documentable: Documentable): Dokka
fun PreMergeDocumentableTransformer.perPackageOptions(documentable: Documentable): PackageOptions? {
val packageName = documentable.dri.packageName ?: return null
return sourceSet(documentable).perPackageOptions
.sortedByDescending { packageOptions -> packageOptions.prefix.length }
.firstOrNull { packageOptions -> packageName.startsWith(packageOptions.prefix) }
.sortedByDescending { packageOptions -> packageOptions.matchingRegex.length }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This feels a bit awkward with regexes. I think I would prefer the order of the rules in the Gradle file to determine priority but I'll let people more familiar with the subject decide.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I think ordering in Gradle might be more problematic and potentially could break easier. Also it will cause some confusion especially when having split logic for dokka configuration (eg. some packageOptions in buildSrc and some in Gradle build files), so I'd personally stick with the regex for now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair point about split logic, especially since it's hard to know in advance the order of execution of Gradle scripts. I added a small note to the Gradle documentation to tell users that the longuest matchingRegexp will be used.

For the cli usage, maybe the order of arguments would still make sense? Or ultimately, maybe an explicit priority option will be needed. But this can be all be added later depending the needs and use cases

.firstOrNull { packageOptions -> Regex(packageOptions.matchingRegex).matches(packageName) }
}

fun <T> PreMergeDocumentableTransformer.source(documentable: T) where T : Documentable, T : WithSources =
Expand Down
2 changes: 1 addition & 1 deletion docs/src/doc/docs/user_guide/cli/usage.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ Dokka supports the following command line arguments:
* `-skipDeprecated` - if set, deprecated elements are not included in the generated documentation
* `-reportUndocumented` - warn about undocumented members
* `-skipEmptyPackages` - do not create index pages for empty packages
* `-packageOptions` - list of package options in format `prefix,-deprecated,-privateApi,+reportUndocumented;prefix, ...`, separated by `;`
* `-packageOptions` - list of package options in format `matchingRegex,-deprecated,-privateApi,+reportUndocumented;matchingRegex, ...`, separated by `;`
* `-links` - list of external documentation links in format `url^packageListUrl^^url2...`, separated by `;`
* `-srcLink` - mapping between a source directory and a Web site for browsing the code in format `<path>=<url>[#lineSuffix]`
* `-noStdlibLink` - disable linking to online kotlin-stdlib documentation
Expand Down
5 changes: 3 additions & 2 deletions docs/src/doc/docs/user_guide/gradle/usage.md
Original file line number Diff line number Diff line change
Expand Up @@ -169,16 +169,17 @@ dokkaHtml {

// Allows to customize documentation generation options on a per-package basis
// Repeat for multiple packageOptions
// If multiple packages match the same matchingRegex, the longuest matchingRegex will be used
perPackageOption {
prefix.set("kotlin") // will match kotlin and all sub-packages of it
matchingRegex.set("kotlin($|\\.).*") // will match kotlin and all sub-packages of it
// All options are optional, default values are below:
skipDeprecated.set(false)
reportUndocumented.set(true) // Emit warnings about not documented members
includeNonPublic.set(false)
}
// Suppress a package
perPackageOption {
prefix.set("kotlin.internal") // will match kotlin.internal and all sub-packages of it
matchingRegex.set(".*\.internal.*") // will match all .internal packages and sub-packages
suppress.set(true)
}
}
Expand Down
2 changes: 1 addition & 1 deletion docs/src/doc/docs/user_guide/maven/usage.md
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ The available configuration options are shown below:
<perPackageOptions>
<packageOptions>
<!-- Will match kotlin and all sub-packages of it -->
<prefix>kotlin</prefix>
<matchingRegex>kotlin($|\.).*</matchingRegex>

<!-- All options are optional, default values are below: -->
<skipDeprecated>false</skipDeprecated>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class DeprecatedDocumentableFilterTransformer(val context: DokkaContext) : PreMe
fun <T> T.isAllowedInPackage(): Boolean where T : WithExtraProperties<T>, T : Documentable {
val packageName = this.dri.packageName
val condition = packageName != null && packageOptions.firstOrNull {
packageName.startsWith(it.prefix)
Regex(it.matchingRegex).matches(packageName)
}?.skipDeprecated
?: globalOptions.skipDeprecated

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class DocumentableVisibilityFilterTransformer(val context: DokkaContext) : PreMe
is JavaVisibility.Default,
is KotlinVisibility.Public -> true
else -> packageName != null
&& packageOptions.firstOrNull { packageName.startsWith(it.prefix) }?.includeNonPublic
&& packageOptions.firstOrNull { Regex(it.matchingRegex).matches(packageName) }?.includeNonPublic
?: globalOptions.includeNonPublic
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ internal class ReportUndocumentedTransformer : DocumentableTransformer {
): DokkaConfiguration.PackageOptions? {
val packageName = documentable.dri.packageName ?: return null
return dokkaSourceSet.perPackageOptions
.filter { packageOptions -> packageName.startsWith(packageOptions.prefix) }
.maxBy { packageOptions -> packageOptions.prefix.length }
.filter { packageOptions -> Regex(packageOptions.matchingRegex).matches(packageName) }
.maxBy { packageOptions -> packageOptions.matchingRegex.length }
}
}
6 changes: 4 additions & 2 deletions plugins/base/src/test/kotlin/filter/DeprecationFilterTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,13 @@ class DeprecationFilterTest : AbstractCoreTest() {
sourceRoots = listOf("src/main/kotlin/basic/Test.kt")
skipDeprecated = false
perPackageOptions = mutableListOf(
PackageOptionsImpl("example",
PackageOptionsImpl(
"example.*",
true,
false,
true,
false)
false
)
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ class ReportUndocumentedTransformerTest : AbstractCoreTest() {
sourceSets {
sourceSet {
perPackageOptions += packageOptions(
prefix = "sample",
matchingRegex = "sample.*",
reportUndocumented = true,
)
reportUndocumented = false
Expand Down Expand Up @@ -410,11 +410,11 @@ class ReportUndocumentedTransformerTest : AbstractCoreTest() {
sourceSets {
sourceSet {
perPackageOptions += packageOptions(
prefix = "sample",
matchingRegex = "sample.*",
reportUndocumented = false,
)
perPackageOptions += packageOptions(
prefix = "sample.enabled",
matchingRegex = "sample.enabled.*",
reportUndocumented = true,
)
reportUndocumented = false
Expand Down Expand Up @@ -448,11 +448,11 @@ class ReportUndocumentedTransformerTest : AbstractCoreTest() {
sourceSets {
sourceSet {
perPackageOptions += packageOptions(
prefix = "sample",
matchingRegex = "sample.*",
reportUndocumented = true,
)
perPackageOptions += packageOptions(
prefix = "sample.disabled",
matchingRegex = "sample.disabled.*",
reportUndocumented = false,
)
reportUndocumented = true
Expand Down Expand Up @@ -904,13 +904,13 @@ class ReportUndocumentedTransformerTest : AbstractCoreTest() {
}

private fun packageOptions(
prefix: String,
matchingRegex: String,
reportUndocumented: Boolean?,
includeNonPublic: Boolean = true,
skipDeprecated: Boolean = false,
suppress: Boolean = false
) = PackageOptionsImpl(
prefix = prefix,
matchingRegex = matchingRegex,
reportUndocumented = reportUndocumented,
includeNonPublic = includeNonPublic,
skipDeprecated = skipDeprecated,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ class SuppressedDocumentableFilterTransformerTest : AbstractCoreTest() {
sourceSet {
sourceRoots = listOf("src")
perPackageOptions = listOf(
packageOptions(prefix = "suppressed", suppress = true),
packageOptions(prefix = "default", suppress = false)
packageOptions(matchingRegex = "suppressed.*", suppress = true),
packageOptions(matchingRegex = "default.*", suppress = false)
)
}
}
Expand Down Expand Up @@ -57,11 +57,11 @@ class SuppressedDocumentableFilterTransformerTest : AbstractCoreTest() {
sourceSet {
sourceRoots = listOf("src")
perPackageOptions = listOf(
packageOptions(prefix = "parent.some", suppress = false),
packageOptions(prefix = "parent.some.suppressed", suppress = true),
packageOptions(matchingRegex = "parent.some.*", suppress = false),
packageOptions(matchingRegex = "parent.some.suppressed.*", suppress = true),

packageOptions(prefix = "parent.other", suppress = true),
packageOptions(prefix = "parent.other.default", suppress = false)
packageOptions(matchingRegex = "parent.other.*", suppress = true),
packageOptions(matchingRegex = "parent.other.default.*", suppress = false)
)
}
}
Expand Down Expand Up @@ -175,10 +175,10 @@ class SuppressedDocumentableFilterTransformerTest : AbstractCoreTest() {
}

private fun packageOptions(
prefix: String,
matchingRegex: String,
suppress: Boolean
) = PackageOptionsImpl(
prefix = prefix,
matchingRegex = matchingRegex,
suppress = suppress,
includeNonPublic = true,
reportUndocumented = false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ class GradlePackageOptionsBuilder(
@Transient @get:Internal internal val project: Project
) : DokkaConfigurationBuilder<PackageOptionsImpl> {
@Input
val prefix: Property<String> = project.objects.safeProperty<String>()
.safeConvention("")
val matchingRegex: Property<String> = project.objects.safeProperty<String>()
.safeConvention(".*")

@Input
val includeNonPublic: Property<Boolean> = project.objects.safeProperty<Boolean>()
Expand All @@ -35,7 +35,7 @@ class GradlePackageOptionsBuilder(
.safeConvention(DokkaDefaults.suppress)

override fun build(): PackageOptionsImpl = PackageOptionsImpl(
prefix = checkNotNull(prefix.getSafe()) { "prefix not specified" },
matchingRegex = checkNotNull(matchingRegex.getSafe()) { "prefix not specified" },
includeNonPublic = includeNonPublic.getSafe(),
reportUndocumented = reportUndocumented.getSafe(),
skipDeprecated = skipDeprecated.getSafe(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,21 +245,21 @@ class GradleDokkaSourceSetBuilderTest {
assertEquals(emptyList(), sourceSet.build().perPackageOptions, "Expected no default per package options")

sourceSet.perPackageOptions.add(GradlePackageOptionsBuilder(project).apply {
this.prefix by "p1"
this.matchingRegex by "p1.*"
})

sourceSet.perPackageOption {
it.prefix by "p2"
it.matchingRegex by "p2.*"
}

sourceSet.perPackageOption(project.closureOf<GradlePackageOptionsBuilder> {
this.prefix by "p3"
this.matchingRegex by "p3.*"
})

assertEquals(
listOf("p1", "p2", "p3").map { prefix ->
listOf("p1.*", "p2.*", "p3.*").map { matchingRegex ->
PackageOptionsImpl(
prefix = prefix,
matchingRegex = matchingRegex,
includeNonPublic = DokkaDefaults.includeNonPublic,
reportUndocumented = DokkaDefaults.reportUndocumented,
skipDeprecated = DokkaDefaults.skipDeprecated,
Expand Down
4 changes: 2 additions & 2 deletions runners/maven-plugin/src/main/kotlin/DokkaMojo.kt
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ abstract class AbstractDokkaMojo(private val defaultDokkaPlugins: List<Dependenc

class PackageOptions : DokkaConfiguration.PackageOptions {
@Parameter
override var prefix: String = ""
override var matchingRegex: String = ".*"

@Parameter
override var includeNonPublic: Boolean = DokkaDefaults.includeNonPublic
Expand Down Expand Up @@ -203,7 +203,7 @@ abstract class AbstractDokkaMojo(private val defaultDokkaPlugins: List<Dependenc
sourceLinks = sourceLinks.map { SourceLinkDefinitionImpl(it.path, URL(it.url), it.lineSuffix) }.toSet(),
perPackageOptions = perPackageOptions.map {
PackageOptionsImpl(
prefix = it.prefix,
matchingRegex = it.matchingRegex,
includeNonPublic = it.includeNonPublic,
reportUndocumented = it.reportUndocumented,
skipDeprecated = it.skipDeprecated,
Expand Down