Skip to content

Commit 9f31cda

Browse files
rlirli
authored
Cleanup obvious quality issues in CodeScan (#4811)
1 parent 4886406 commit 9f31cda

File tree

15 files changed

+239
-210
lines changed

15 files changed

+239
-210
lines changed

‎gradle/libs.versions.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ jackson = "2.16.1"
1414
jacoco = "0.8.12"
1515
jgit = "6.5.0.202303070854-r"
1616
junit4 = "4.13.2"
17-
junit5 = "5.10.3"
17+
junit5 = "5.11.0"
1818
# https://plugins.jetbrains.com/docs/intellij/kotlin.html#adding-kotlin-support
1919
# https://kotlinlang.org/docs/releases.html#release-details
2020
kotlin = "2.0.0"

‎plugins/amazonq/chat/jetbrains-community/src/software/aws/toolkits/jetbrains/services/cwc/editor/context/project/EncoderServer.kt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ class EncoderServer(val project: Project) : Disposable {
105105
return true
106106
}
107107
} catch (e: Exception) {
108-
logger.warn { "error running encoder server: ${e.stackTraceToString()}" }
108+
logger.warn(e) { "error running encoder server:" }
109109
processHandler?.destroyProcess()
110110
numberOfRetry.incrementAndGet()
111111
return false
@@ -183,7 +183,7 @@ class EncoderServer(val project: Project) : Disposable {
183183
}
184184
}
185185
} catch (e: Exception) {
186-
logger.warn { "error downloading artifacts ${e.stackTraceToString()}" }
186+
logger.warn(e) { "error downloading artifacts:" }
187187
}
188188
}
189189

@@ -200,7 +200,7 @@ class EncoderServer(val project: Project) : Disposable {
200200
}
201201
}
202202
} catch (e: Exception) {
203-
logger.warn { "error deleting old artifacts ${e.stackTraceToString()}" }
203+
logger.warn(e) { "error deleting old artifacts:" }
204204
}
205205
}
206206

‎plugins/amazonq/codetransform/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codemodernizer/CodeModernizerManager.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -560,7 +560,7 @@ class CodeModernizerManager(private val project: Project) : PersistentStateCompo
560560
LOG.error { "Unable to resume job as credentials are invalid" }
561561
// User is logged in with old or invalid credentials, nothing to do until they log in with valid credentials
562562
} catch (e: Exception) {
563-
LOG.error { "Unable to resume job as an unexpected exception occurred ${e.stackTraceToString()}" }
563+
LOG.error(e) { "Unable to resume job as an unexpected exception occurred" }
564564
} finally {
565565
isResumingJob.set(false)
566566
}

‎plugins/amazonq/codetransform/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codemodernizer/utils/CodeTransformApiUtils.kt‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package software.aws.toolkits.jetbrains.services.codemodernizer.utils
55

66
import com.intellij.openapi.project.Project
77
import com.intellij.serviceContainer.AlreadyDisposedException
8+
import kotlinx.coroutines.delay
89
import software.amazon.awssdk.awscore.exception.AwsServiceException
910
import software.amazon.awssdk.services.codewhispererruntime.model.AccessDeniedException
1011
import software.amazon.awssdk.services.codewhispererruntime.model.CodeWhispererRuntimeException
@@ -24,7 +25,6 @@ import software.aws.toolkits.jetbrains.services.codemodernizer.client.GumbyClien
2425
import software.aws.toolkits.jetbrains.services.codemodernizer.constants.BILLING_RATE
2526
import software.aws.toolkits.jetbrains.services.codemodernizer.model.JobId
2627
import software.aws.toolkits.resources.message
27-
import java.lang.Thread.sleep
2828
import java.time.Duration
2929
import java.util.Locale
3030
import java.util.concurrent.atomic.AtomicBoolean
@@ -78,15 +78,15 @@ suspend fun JobId.pollTransformationStatusAndPlan(
7878
) {
7979
try {
8080
if (!didSleepOnce) {
81-
sleep(initialSleepDurationMillis)
81+
delay(initialSleepDurationMillis)
8282
didSleepOnce = true
8383
}
8484
if (isDisposed.get()) throw AlreadyDisposedException("The invoker is disposed.")
8585
transformationResponse = clientAdaptor.getCodeModernizationJob(this.id)
8686
val newStatus = transformationResponse?.transformationJob()?.status() ?: throw RuntimeException("Unable to get job status")
8787
var newPlan: TransformationPlan? = null
8888
if (newStatus in STATES_WHERE_PLAN_EXIST) {
89-
sleep(sleepDurationMillis)
89+
delay(sleepDurationMillis)
9090
newPlan = clientAdaptor.getCodeModernizationPlan(this).transformationPlan()
9191
}
9292
if (newStatus != state) {
@@ -111,7 +111,7 @@ suspend fun JobId.pollTransformationStatusAndPlan(
111111
refreshToken(project)
112112
return@waitUntil state
113113
} finally {
114-
sleep(sleepDurationMillis)
114+
delay(sleepDurationMillis)
115115
}
116116
}
117117
} catch (e: Exception) {

‎plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/codescan/CodeWhispererCodeScanManager.kt‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ import software.aws.toolkits.core.utils.debug
5050
import software.aws.toolkits.core.utils.error
5151
import software.aws.toolkits.core.utils.getLogger
5252
import software.aws.toolkits.core.utils.info
53+
import software.aws.toolkits.core.utils.warn
5354
import software.aws.toolkits.jetbrains.core.coroutines.getCoroutineUiContext
5455
import software.aws.toolkits.jetbrains.core.coroutines.projectCoroutineScope
5556
import software.aws.toolkits.jetbrains.core.credentials.ToolkitConnectionManager
@@ -85,6 +86,8 @@ import javax.swing.tree.DefaultMutableTreeNode
8586
import javax.swing.tree.TreePath
8687
import kotlin.coroutines.CoroutineContext
8788

89+
private val LOG = getLogger<CodeWhispererCodeScanManager>()
90+
8891
class CodeWhispererCodeScanManager(val project: Project) {
8992
private val codeScanResultsPanel by lazy {
9093
CodeWhispererCodeScanResultsView(project)
@@ -220,7 +223,6 @@ class CodeWhispererCodeScanManager(val project: Project) {
220223
val startTime = Instant.now().toEpochMilli()
221224
var codeScanResponseContext = defaultCodeScanResponseContext()
222225
val connection = ToolkitConnectionManager.getInstance(project).activeConnectionForFeature(CodeWhispererConnection.getInstance())
223-
var codeScanJobId: String? = null
224226
var language: CodeWhispererProgrammingLanguage = CodeWhispererUnknownLanguage.INSTANCE
225227
var skipTelemetry = false
226228
try {
@@ -253,7 +255,6 @@ class CodeWhispererCodeScanManager(val project: Project) {
253255
language = codeScanSessionConfig.getProgrammingLanguage()
254256
if (language == CodeWhispererPlainText.INSTANCE) { skipTelemetry = true }
255257
codeScanResponseContext = codeScanResponse.responseContext
256-
codeScanJobId = codeScanResponseContext.codeScanJobId
257258
when (codeScanResponse) {
258259
is CodeScanResponse.Success -> {
259260
val issues = codeScanResponse.issues
@@ -273,7 +274,7 @@ class CodeWhispererCodeScanManager(val project: Project) {
273274
throw codeScanResponse.failureReason
274275
}
275276
}
276-
LOG.info { "Security scan completed for jobID: $codeScanJobId." }
277+
LOG.info { "Security scan completed for jobID: ${codeScanResponseContext.codeScanJobId}." }
277278
}
278279
}
279280
} catch (e: Error) {
@@ -304,7 +305,7 @@ class CodeWhispererCodeScanManager(val project: Project) {
304305
scope
305306
)
306307
)
307-
sendCodeScanTelemetryToServiceAPI(project, language, codeScanJobId, scope)
308+
sendCodeScanTelemetryToServiceAPI(project, language, codeScanResponseContext.codeScanJobId, scope)
308309
}
309310
}
310311
}
@@ -377,12 +378,11 @@ class CodeWhispererCodeScanManager(val project: Project) {
377378
}
378379

379380
if (scope == CodeWhispererConstants.CodeAnalysisScope.PROJECT) {
380-
LOG.error {
381+
LOG.error(e) {
381382
"Failed to run security scan and display results. Caused by: $errorMessage, status code: $errorCode, " +
382383
"exception: ${e::class.simpleName}, request ID: $requestId " +
383384
"Jetbrains IDE: ${ApplicationInfo.getInstance().fullApplicationName}, " +
384-
"IDE version: ${ApplicationInfo.getInstance().apiVersion}, " +
385-
"stacktrace: ${e.stackTrace.contentDeepToString()}"
385+
"IDE version: ${ApplicationInfo.getInstance().apiVersion}, "
386386
}
387387
}
388388

@@ -704,7 +704,6 @@ class CodeWhispererCodeScanManager(val project: Project) {
704704
}
705705

706706
companion object {
707-
private val LOG = getLogger<CodeWhispererCodeScanManager>()
708707
fun getInstance(project: Project): CodeWhispererCodeScanManager = project.service()
709708
}
710709
}
@@ -765,7 +764,11 @@ data class CodeWhispererCodeScanIssue(
765764
markupModel: MarkupModel? =
766765
DocumentMarkupModel.forDocument(document, project, false)
767766
): RangeHighlighterEx? {
768-
if (!ApplicationManager.getApplication().isDispatchThread) return null
767+
if (!ApplicationManager.getApplication().isDispatchThread) {
768+
LOG.warn(RuntimeException()) { "Attempted to call addRangeHighlighter on EDT" }
769+
return null
770+
}
771+
769772
return markupModel?.let {
770773
textRange ?: return null
771774
it.addRangeHighlighter(

‎plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/codescan/CodeWhispererCodeScanSession.kt‎

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ import com.intellij.openapi.application.runReadAction
1212
import com.intellij.openapi.fileEditor.FileDocumentManager
1313
import com.intellij.openapi.project.Project
1414
import com.intellij.openapi.vfs.LocalFileSystem
15-
import com.intellij.util.TimeoutUtil.sleep
1615
import com.intellij.util.io.HttpRequests
16+
import kotlinx.coroutines.delay
1717
import kotlinx.coroutines.ensureActive
1818
import kotlinx.coroutines.isActive
1919
import kotlinx.coroutines.time.withTimeout
@@ -89,7 +89,6 @@ class CodeWhispererCodeScanSession(val sessionContext: CodeScanSessionContext) {
8989
* 6. Return the results from the ListCodeScan API.
9090
*/
9191
suspend fun run(): CodeScanResponse {
92-
var issues: List<CodeWhispererCodeScanIssue> = listOf()
9392
var codeScanResponseContext = defaultCodeScanResponseContext()
9493
val currentCoroutineContext = coroutineContext
9594
try {
@@ -157,9 +156,9 @@ class CodeWhispererCodeScanSession(val sessionContext: CodeScanSessionContext) {
157156
val jobId = createCodeScanResponse.jobId()
158157
codeScanResponseContext = codeScanResponseContext.copy(codeScanJobId = jobId)
159158
if (isProjectScope()) {
160-
sleep(PROJECT_SCAN_INITIAL_POLLING_INTERVAL_IN_SECONDS * TOTAL_MILLIS_IN_SECOND)
159+
delay(PROJECT_SCAN_INITIAL_POLLING_INTERVAL_IN_SECONDS * TOTAL_MILLIS_IN_SECOND)
161160
} else {
162-
sleep(FILE_SCAN_INITIAL_POLLING_INTERVAL_IN_SECONDS * TOTAL_MILLIS_IN_SECOND)
161+
delay(FILE_SCAN_INITIAL_POLLING_INTERVAL_IN_SECONDS * TOTAL_MILLIS_IN_SECOND)
163162
}
164163

165164
// 5. Keep polling the API GetCodeScan to wait for results for a given timeout period.
@@ -180,7 +179,7 @@ class CodeWhispererCodeScanSession(val sessionContext: CodeScanSessionContext) {
180179
"request id: ${getCodeScanResponse.responseMetadata().requestId()}"
181180
}
182181
}
183-
sleepThread()
182+
delay(CODE_SCAN_POLLING_INTERVAL_IN_SECONDS * TOTAL_MILLIS_IN_SECOND)
184183
if (codeScanStatus == CodeScanStatus.FAILED) {
185184
if (isProjectScope()) {
186185
LOG.debug {
@@ -217,7 +216,7 @@ class CodeWhispererCodeScanSession(val sessionContext: CodeScanSessionContext) {
217216
LOG.debug { "Rendering response to display security scan results." }
218217
}
219218
currentCoroutineContext.ensureActive()
220-
issues = mapToCodeScanIssues(documents)
219+
val issues = mapToCodeScanIssues(documents)
221220
codeScanResponseContext = codeScanResponseContext.copy(codeScanTotalIssues = issues.count())
222221
codeScanResponseContext = codeScanResponseContext.copy(codeScanIssuesWithFixes = issues.count { it.suggestedFixes.isNotEmpty() })
223222
codeScanResponseContext = codeScanResponseContext.copy(reason = "Succeeded")
@@ -237,14 +236,13 @@ class CodeWhispererCodeScanSession(val sessionContext: CodeScanSessionContext) {
237236
}
238237
}
239238
}
240-
LOG.error {
239+
LOG.error(e) {
241240
"Failed to run security scan and display results. Caused by: ${e.message}, status code: ${awsError?.errorCode()}, " +
242241
"exception: ${e::class.simpleName}, request ID: ${exception?.requestId()}" +
243242
"Jetbrains IDE: ${ApplicationInfo.getInstance().fullApplicationName}, " +
244-
"IDE version: ${ApplicationInfo.getInstance().apiVersion}, " +
245-
"stacktrace: ${e.stackTrace.contentDeepToString()}"
243+
"IDE version: ${ApplicationInfo.getInstance().apiVersion}, "
246244
}
247-
return CodeScanResponse.Failure(issues, codeScanResponseContext, e)
245+
return CodeScanResponse.Failure(codeScanResponseContext, e)
248246
}
249247
}
250248

@@ -445,10 +443,6 @@ class CodeWhispererCodeScanSession(val sessionContext: CodeScanSessionContext) {
445443
}
446444
}
447445

448-
fun sleepThread() {
449-
sleep(CODE_SCAN_POLLING_INTERVAL_IN_SECONDS * TOTAL_MILLIS_IN_SECOND)
450-
}
451-
452446
fun getTelemetryErrorMessage(e: Exception): String = when {
453447
e.message?.contains("Resource not found.") == true -> "Resource not found."
454448
e.message?.contains("Service returned HTTP status code 407") == true -> "Service returned HTTP status code 407"
@@ -478,16 +472,15 @@ class CodeWhispererCodeScanSession(val sessionContext: CodeScanSessionContext) {
478472
}
479473

480474
sealed class CodeScanResponse {
481-
abstract val issues: List<CodeWhispererCodeScanIssue>
482475
abstract val responseContext: CodeScanResponseContext
483476

484477
data class Success(
485-
override val issues: List<CodeWhispererCodeScanIssue>,
478+
val issues: List<CodeWhispererCodeScanIssue>,
486479
override val responseContext: CodeScanResponseContext
487480
) : CodeScanResponse()
488481

489482
data class Failure(
490-
override val issues: List<CodeWhispererCodeScanIssue>,
483+
// this needs some cleanup as it solely exists for tracking the job ID
491484
override val responseContext: CodeScanResponseContext,
492485
val failureReason: Throwable
493486
) : CodeScanResponse()

‎plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/codescan/listeners/CodeWhispererCodeScanEditorMouseMotionListener.kt‎

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,8 @@ class CodeWhispererCodeScanEditorMouseMotionListener(private val project: Projec
8383
private val issueDataKey = DataKey.create<MutableMap<String, String>>("amazonq.codescan.explainissue")
8484

8585
private fun getHtml(issue: CodeWhispererCodeScanIssue): String {
86-
val isFixAvailable = issue.suggestedFixes.isNotEmpty()
86+
// not sure why service team allows multiple remediations, but we only show one
87+
val suggestedFix = issue.suggestedFixes.firstOrNull()
8788

8889
val cweLinks = if (issue.relatedVulnerabilities.isNotEmpty()) {
8990
issue.relatedVulnerabilities.joinToString(", ") { cwe ->
@@ -111,7 +112,7 @@ class CodeWhispererCodeScanEditorMouseMotionListener(private val project: Projec
111112
<tbody>
112113
<tr>
113114
<td>$cweLinks</td>
114-
<td>${if (isFixAvailable) "<span style=\"color:${additionForegroundColor.getHexString()};\">Yes</span>" else "<span style=\"color:${deletionForegroundColor.getHexString()};\">No</span>"}</td>
115+
<td>${if (suggestedFix != null) "<span style=\"color:${additionForegroundColor.getHexString()};\">Yes</span>" else "<span style=\"color:${deletionForegroundColor.getHexString()};\">No</span>"}</td>
115116
<td>$detectorLibraryLink</td>
116117
</tr>
117118
</tbody>
@@ -128,17 +129,17 @@ class CodeWhispererCodeScanEditorMouseMotionListener(private val project: Projec
128129
<br />
129130
""".trimIndent()
130131

131-
val suggestedFixSection = if (isFixAvailable) {
132-
val isFixDescriptionAvailable = issue.suggestedFixes[0].description.isNotBlank() &&
133-
issue.suggestedFixes[0].description.trim() != "Suggested remediation:"
132+
val suggestedFixSection = suggestedFix?.let {
133+
val isFixDescriptionAvailable = it.description.isNotBlank() &&
134+
it.description.trim() != "Suggested remediation:"
134135
"""
135136
|<hr />
136137
|<br />
137138
|
138139
|## ${message("codewhisperer.codescan.suggested_fix_label")}
139140
|
140141
|```diff
141-
|${issue.suggestedFixes[0].code}
142+
|${it.code}
142143
|```
143144
|
144145
|${
@@ -147,14 +148,12 @@ class CodeWhispererCodeScanEditorMouseMotionListener(private val project: Projec
147148
message(
148149
"codewhisperer.codescan.suggested_fix_description"
149150
)
150-
}\n${issue.suggestedFixes[0].description}"
151+
}\n${it.description}"
151152
} else {
152153
""
153154
}
154155
}
155156
""".trimMargin()
156-
} else {
157-
""
158157
}
159158

160159
return convertMarkdownToHTML(
@@ -165,7 +164,7 @@ class CodeWhispererCodeScanEditorMouseMotionListener(private val project: Projec
165164
|
166165
|$detectorSection
167166
|
168-
|$suggestedFixSection
167+
|${suggestedFixSection.orEmpty()}
169168
""".trimMargin()
170169
)
171170
}
@@ -180,9 +179,9 @@ class CodeWhispererCodeScanEditorMouseMotionListener(private val project: Projec
180179
}
181180

182181
private fun showPopup(issues: List<CodeWhispererCodeScanIssue>, e: EditorMouseEvent, issueIndex: Int = 0) {
183-
if (issues == null || issues.isEmpty()) {
182+
if (issues.isEmpty()) {
184183
LOG.debug {
185-
"Unable to show popup issue at ${e.logicalPosition} as the issue was null"
184+
"Unable to show popup issue at ${e.logicalPosition} as there are no issues"
186185
}
187186
return
188187
}
@@ -396,10 +395,6 @@ class CodeWhispererCodeScanEditorMouseMotionListener(private val project: Projec
396395

397396
val documentContent = document.text
398397
val updatedContent = applyPatch(issue.suggestedFixes[0].code, documentContent, issue.file.name)
399-
if (updatedContent == null) {
400-
// println(applyPatchToContent(issue.suggestedFixes[0].code, documentContent))
401-
throw Error("Patch apply failed.")
402-
}
403398
document.replaceString(document.getLineStartOffset(0), document.getLineEndOffset(document.lineCount - 1), updatedContent)
404399
PsiDocumentManager.getInstance(issue.project).commitDocument(document)
405400
CodeWhispererTelemetryService.getInstance().sendCodeScanIssueApplyFixEvent(issue, Result.Succeeded)

‎plugins/amazonq/codewhisperer/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/codewhisperer/CodeWhispererTestUtil.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,10 @@ import com.intellij.openapi.editor.VisualPosition
77
import com.intellij.openapi.project.Project
88
import kotlinx.coroutines.async
99
import kotlinx.coroutines.runBlocking
10-
import org.gradle.internal.impldep.com.amazonaws.ResponseMetadata.AWS_REQUEST_ID
1110
import org.mockito.kotlin.mock
1211
import software.amazon.awssdk.awscore.DefaultAwsResponseMetadata
1312
import software.amazon.awssdk.awscore.exception.AwsErrorDetails
13+
import software.amazon.awssdk.awscore.util.AwsHeader
1414
import software.amazon.awssdk.http.SdkHttpResponse
1515
import software.amazon.awssdk.services.codewhispererruntime.model.CodeWhispererRuntimeException
1616
import software.amazon.awssdk.services.codewhispererruntime.model.Completion
@@ -74,7 +74,7 @@ object CodeWhispererTestUtil {
7474
Pair("BSD-4-Clause", "testRepo3")
7575
)
7676
val metadata: DefaultAwsResponseMetadata = DefaultAwsResponseMetadata.create(
77-
mapOf(AWS_REQUEST_ID to testRequestId)
77+
mapOf(AwsHeader.AWS_REQUEST_ID to testRequestId)
7878
)
7979
val sdkHttpResponse = SdkHttpResponse.builder().headers(
8080
mapOf(CodeWhispererService.KET_SESSION_ID to listOf(testSessionId))

0 commit comments

Comments
 (0)