refactor: Harden detekt setup, address some issues (#507)

# Detekt setup hardeing; address some issues

- Address some detekt findings: missing KDocs, formatting, use `check*`
& `orEmpty()``
- Remove unnecessary KDoc comments from deprecated classes in `types.kt`
- Add Detekt baseline XML files for all modules
- Enforce stricter Detekt rules by setting `failOnSeverity` from `None`
to `Error`

## Motivation and Context

Detekt is not actually failing the build on violations. This PR is
enabling Detekt to fail on errors. Baseline fines added for historical
issues. Follow-up for #501

## How Has This Been Tested?
Regressions

## Breaking Changes

No breaking changes.

## Types of changes
<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] Refactoring
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [x] Documentation update

## Checklist
<!-- Go over all the following points, and put an `x` in all the boxes
that apply. -->
- [x] I have read the [MCP
Documentation](https://modelcontextprotocol.io)
- [x] My code follows the repository's style guidelines
- [x] New and existing tests pass locally
- [ ] I have added appropriate error handling
- [ ] I have added or updated documentation as needed
This commit is contained in:
Konstantin Pavlov 2026-02-13 22:17:27 +02:00 • committed by GitHub
commit ed9ad83290
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 478 additions and 122 deletions

View file

@ -0,0 +1,11 @@
<?xml version='1.0' encoding='UTF-8'?>
<SmellBaseline>
<ManuallySuppressedIssues/>
<CurrentIssues>
<ID>EmptyFunctionBlock:ClientTest.kt:ClientTest.&lt;no name provided>${ }</ID>
<ID>EmptyFunctionBlock:ClientTest.kt:ClientTest.&lt;no name provided>${}</ID>
<ID>LargeClass:ClientTest.kt:ClientTest</ID>
<ID>LongMethod:ClientTest.kt:ClientTest$@Test fun `JSONRPCRequest with ToolsList method and default params returns list of tools`</ID>
<ID>LongMethod:ClientTest.kt:ClientTest$@Test fun `should handle logging setLevel request`</ID>
</CurrentIssues>
</SmellBaseline>

View file

@ -0,0 +1,47 @@
<?xml version='1.0' encoding='UTF-8'?>
<SmellBaseline>
<ManuallySuppressedIssues/>
<CurrentIssues>
<ID>AbstractClassCanBeConcreteClass:AbstractPromptIntegrationTest.kt:AbstractPromptIntegrationTest$AbstractPromptIntegrationTest</ID>
<ID>AbstractClassCanBeConcreteClass:AbstractResourceIntegrationTest.kt:AbstractResourceIntegrationTest$AbstractResourceIntegrationTest</ID>
<ID>AbstractClassCanBeConcreteClass:AbstractToolIntegrationTest.kt:AbstractToolIntegrationTest$AbstractToolIntegrationTest</ID>
<ID>AbstractClassCanBeConcreteClass:BaseTransportTest.kt:BaseTransportTest$BaseTransportTest</ID>
<ID>CyclomaticComplexMethod:AbstractToolIntegrationTest.kt:AbstractToolIntegrationTest$private fun setupCalculatorTool</ID>
<ID>EmptyFunctionBlock:ClientTest.kt:ClientTest.&lt;no name provided>${ }</ID>
<ID>EmptyFunctionBlock:ClientTest.kt:ClientTest.&lt;no name provided>${}</ID>
<ID>ForbiddenComment:StdioClientTransportTest.kt:StdioClientTransportTest$// TODO: fix running on windows</ID>
<ID>InjectDispatcher:AbstractKotlinClientTsServerTest.kt:AbstractKotlinClientTsServerTest$IO</ID>
<ID>InjectDispatcher:AbstractPromptIntegrationTest.kt:AbstractPromptIntegrationTest$IO</ID>
<ID>InjectDispatcher:AbstractResourceIntegrationTest.kt:AbstractResourceIntegrationTest$IO</ID>
<ID>InjectDispatcher:AbstractToolIntegrationTest.kt:AbstractToolIntegrationTest$IO</ID>
<ID>InjectDispatcher:KotlinClientTsServerEdgeCasesTestSse.kt:KotlinClientTsServerEdgeCasesTestSse$IO</ID>
<ID>InjectDispatcher:KotlinClientTsServerEdgeCasesTestStdio.kt:KotlinClientTsServerEdgeCasesTestStdio$IO</ID>
<ID>InjectDispatcher:SseIntegrationTest.kt:SseIntegrationTest$IO</ID>
<ID>InjectDispatcher:StdioClientTransportTest.kt:StdioClientTransportTest$IO</ID>
<ID>InjectDispatcher:StreamableHttpIntegrationTest.kt:StreamableHttpIntegrationTest$IO</ID>
<ID>InjectDispatcher:TsEdgeCasesTestSse.kt:TsEdgeCasesTestSse$IO</ID>
<ID>InjectDispatcher:WebSocketIntegrationTest.kt:WebSocketIntegrationTest$IO</ID>
<ID>LargeClass:AbstractToolIntegrationTest.kt:AbstractToolIntegrationTest : KotlinTestBase</ID>
<ID>LargeClass:ClientTest.kt:ClientTest</ID>
<ID>LongMethod:AbstractPromptIntegrationTest.kt:AbstractPromptIntegrationTest$@Test fun testMissingRequiredArguments</ID>
<ID>LongMethod:AbstractPromptIntegrationTest.kt:AbstractPromptIntegrationTest$override fun configureServer</ID>
<ID>LongMethod:AbstractResourceIntegrationTest.kt:AbstractResourceIntegrationTest$override fun configureServer</ID>
<ID>LongMethod:AbstractToolIntegrationTest.kt:AbstractToolIntegrationTest$private fun setupCalculatorTool</ID>
<ID>LongMethod:ClientTest.kt:ClientTest$@Test fun `JSONRPCRequest with ToolsList method and default params returns list of tools`</ID>
<ID>LongMethod:ClientTest.kt:ClientTest$@Test fun `should handle logging setLevel request`</ID>
<ID>LongMethod:KotlinServerForTsClientSse.kt:HttpServerTransport$suspend fun handleRequest</ID>
<ID>LongMethod:KotlinServerForTsClientSse.kt:KotlinServerForTsClient$fun createMcpServer: Server</ID>
<ID>LongMethod:KotlinServerForTsClientSse.kt:KotlinServerForTsClient$fun start</ID>
<ID>LongMethod:KotlinTestBase.kt:KotlinTestBase$protected fun setupServer</ID>
<ID>LongMethod:ServerResourcesNotificationSubscribeTest.kt:ServerResourcesNotificationSubscribeTest$@Test fun `should send resource notifications`</ID>
<ID>LongMethod:TsEdgeCasesTestSse.kt:TsEdgeCasesTestSse$@Test @Timeout(30, unit = TimeUnit.SECONDS) fun testComplexConcurrentRequests: Unit</ID>
<ID>MatchingDeclarationName:PromptIntegrationTestSse.kt:SchemaPromptIntegrationTestSse : AbstractPromptIntegrationTest</ID>
<ID>SleepInsteadOfDelay:KotlinServerForTsClientSse.kt:KotlinServerForTsClient$sleep(500)</ID>
<ID>ThrowsCount:AbstractPromptIntegrationTest.kt:AbstractPromptIntegrationTest$override fun configureServer</ID>
<ID>UnusedPrivateProperty:KotlinClientTsServerTestSse.kt:KotlinClientTsServerTestSse$private val host = "localhost"</ID>
<ID>UseCheckOrError:ClientTest.kt:ClientTest.&lt;no name provided>$throw IllegalStateException("Test error")</ID>
<ID>UseOrEmpty:AbstractToolIntegrationTest.kt:AbstractToolIntegrationTest$(request.params.arguments?.get("tags") as? JsonArray)?.mapNotNull { (it as? JsonPrimitive)?.content } ?: emptyList()</ID>
<ID>UseRequire:AbstractPromptIntegrationTest.kt:AbstractPromptIntegrationTest$throw IllegalArgumentException("Missing required argument: $argName")</ID>
<ID>VarCouldBeVal:KotlinClientTsServerEdgeCasesTestSse.kt:KotlinClientTsServerEdgeCasesTestSse$private lateinit var client: Client</ID>
</CurrentIssues>
</SmellBaseline>

View file

@ -40,7 +40,7 @@ class InMemoryTransport : AbstractTransport() {
}
override suspend fun send(message: JSONRPCMessage, options: TransportSendOptions?) {
val other = otherTransport ?: throw IllegalStateException("Not connected")
val other = checkNotNull(otherTransport) { "Not connected" }
other._onMessage.invoke(message)
}

View file

@ -266,7 +266,7 @@ abstract class AbstractPromptIntegrationTest : KotlinTestBase() {
),
),
) { request ->
val args = request.params.arguments ?: emptyMap()
val args = request.params.arguments.orEmpty()
val arg1 = args["requiredArg1"] ?: throw IllegalArgumentException(
"Missing required argument: requiredArg1",
)

View file

@ -24,8 +24,10 @@ class ServerPromptsNotificationTest : AbstractServerFeaturesTest() {
fun `addPrompt should send notification`() = runTest {
// Configure notification handler
val notifications = mutableListOf<PromptListChangedNotification>()
client.setNotificationHandler<PromptListChangedNotification>(Method.Defined.NotificationsPromptsListChanged) {
notifications.add(it)
client.setNotificationHandler<PromptListChangedNotification>(
Method.Defined.NotificationsPromptsListChanged,
) { notification ->
notifications.add(notification)
CompletableDeferred(Unit)
}
@ -96,8 +98,10 @@ class ServerPromptsNotificationTest : AbstractServerFeaturesTest() {
fun `notification should not be send when removed prompt does not exists`() = runTest {
// Track notifications
val notifications = mutableListOf<PromptListChangedNotification>()
client.setNotificationHandler<PromptListChangedNotification>(Method.Defined.NotificationsPromptsListChanged) {
notifications.add(it)
client.setNotificationHandler<PromptListChangedNotification>(
Method.Defined.NotificationsPromptsListChanged,
) { notification ->
notifications.add(notification)
CompletableDeferred(Unit)
}

View file

@ -34,7 +34,7 @@ class ServerPromptsTest : AbstractServerFeaturesTest() {
server.addPrompt(testPrompt) {
GetPromptResult(
description = "Test prompt description",
messages = listOf(),
messages = emptyList(),
)
}
@ -70,13 +70,13 @@ class ServerPromptsTest : AbstractServerFeaturesTest() {
server.addPrompt(testPrompt1) {
GetPromptResult(
description = "Test prompt description 1",
messages = listOf(),
messages = emptyList(),
)
}
server.addPrompt(testPrompt2) {
GetPromptResult(
description = "Test prompt description 2",
messages = listOf(),
messages = emptyList(),
)
}

View file

@ -50,12 +50,12 @@ class ServerResourcesNotificationSubscribeTest : AbstractServerFeaturesTest() {
name = "Test Resource 1",
description = "A test resource 1",
mimeType = "text/plain",
) {
) { request ->
ReadResourceResult(
contents = listOf(
TextResourceContents(
text = "Test resource content 1",
uri = it.uri,
uri = request.uri,
mimeType = "text/plain",
),
),
@ -67,12 +67,12 @@ class ServerResourcesNotificationSubscribeTest : AbstractServerFeaturesTest() {
name = "Test Resource 2",
description = "A test resource 2",
mimeType = "text/plain",
) {
) { request ->
ReadResourceResult(
contents = listOf(
TextResourceContents(
text = "Test resource content 2",
uri = it.uri,
uri = request.uri,
mimeType = "text/plain",
),
),
@ -121,8 +121,8 @@ class ServerResourcesNotificationSubscribeTest : AbstractServerFeaturesTest() {
println("Thread ${Thread.currentThread().name} test 1")
// Track notifications
val notifications = ConcurrentLinkedQueue<ResourceUpdatedNotification>()
client.setNotificationHandler<ResourceUpdatedNotification>(NotificationsResourcesUpdated) {
notifications.add(it)
client.setNotificationHandler<ResourceUpdatedNotification>(NotificationsResourcesUpdated) { notification ->
notifications.add(notification)
CompletableDeferred(Unit)
}

View file

@ -26,8 +26,8 @@ class ServerResourcesNotificationTest : AbstractServerFeaturesTest() {
val notifications = mutableListOf<ResourceListChangedNotification>()
client.setNotificationHandler<ResourceListChangedNotification>(
Method.Defined.NotificationsResourcesListChanged,
) {
notifications.add(it)
) { notification ->
notifications.add(notification)
CompletableDeferred(Unit)
}

View file

@ -51,8 +51,10 @@ class ServerToolsNotificationTest : AbstractServerFeaturesTest() {
fun `removeTools should remove multiple tools and send two notifications`() = runTest {
// Configure notification handler
val notifications = mutableListOf<ToolListChangedNotification>()
client.setNotificationHandler<ToolListChangedNotification>(Method.Defined.NotificationsToolsListChanged) {
notifications.add(it)
client.setNotificationHandler<ToolListChangedNotification>(
Method.Defined.NotificationsToolsListChanged,
) { notification ->
notifications.add(notification)
CompletableDeferred(Unit)
}
@ -83,8 +85,10 @@ class ServerToolsNotificationTest : AbstractServerFeaturesTest() {
fun `notification should not be send when removed tool does not exists`() = runTest {
// Track notifications
val notifications = mutableListOf<ToolListChangedNotification>()
client.setNotificationHandler<ToolListChangedNotification>(Method.Defined.NotificationsToolsListChanged) {
notifications.add(it)
client.setNotificationHandler<ToolListChangedNotification>(
Method.Defined.NotificationsToolsListChanged,
) { notification ->
notifications.add(notification)
CompletableDeferred(Unit)
}