-
Notifications
You must be signed in to change notification settings - Fork 35
Hotfix/2.13.1 #707
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
Hotfix/2.13.1 #707
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| ** general ** | ||
| always be concise, direct and don't try to appease me. | ||
|
|
||
| ** how to implement ** | ||
| always write and update test cases. iterate until they pass. | ||
| use existing mocks, don't write new ones. | ||
| if you use mocks, use mockk to generate them. | ||
| always run the tests after editing. | ||
| use junit4 syntax | ||
|
|
||
| ** security ** | ||
| determine the absolute path of the project directory. you can do that e.g. by executing pwd on the shell within the directory. | ||
| always use snyk for sca and code scanning. scan with both tools. you need to pass the absolute path of the directory we are scanning. | ||
| run snyk code tests after each edit. pass the absolute path of the project directory as a parameter | ||
| run snyk sca tests after updating the package manager / manifest file / build file. pass the absolute path of the project directory as a parameter. | ||
| run snyk sca and code test before committing. fix issues before committing. | ||
|
|
||
| ** fixing issues ** | ||
| fix issues if they are fixable. take the snyk scan results and the test results as input. | ||
|
|
||
| ** committing ** | ||
| when asked to commit, always use conventional commit messages (Conventional Commit Style (Subject + Body)). be descriptive in the body. if you find a JIRA issue (IDE-XXXX) in the branch name, use it as a postfix to the subject line in the format [IDE-XXXX] | ||
| when asked to push, always use 'git push --set-upstream origin $(git_current_branch)' with git_current_branch being the current branch we are on | ||
| never force push | ||
| never push without asking | ||
| never commit the hashicorp gomod | ||
| regularly fetch main branch and offer to merge it into git_current_branch | ||
| don't touch the copyright header |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -421,12 +421,17 @@ fun String.toVirtualFile(): VirtualFile { | |
| return if (!this.startsWith("file:")) { | ||
| StandardFileSystems.local().refreshAndFindFileByPath(this) ?: throw FileNotFoundException(this) | ||
| } else { | ||
| val filePath = Paths.get(this.toFilePathString()) | ||
| val filePath = fromUriToPath() | ||
| VirtualFileManager.getInstance().refreshAndFindFileByNioPath(filePath) | ||
| ?: throw FileNotFoundException(this) | ||
| } | ||
| } | ||
|
|
||
| fun String.fromUriToPath(): Path { | ||
| val filePath = Paths.get(URI.create(this)) | ||
| return filePath.normalize() | ||
| } | ||
|
|
||
| fun String.toVirtualFileOrNull(): VirtualFile? { | ||
| return try { | ||
| this.toVirtualFile() | ||
|
|
@@ -436,77 +441,11 @@ fun String.toVirtualFileOrNull(): VirtualFile? { | |
| } | ||
|
|
||
| fun VirtualFile.toLanguageServerURI(): String { | ||
| return this.url.toFileURIString() | ||
| } | ||
|
|
||
| /** | ||
| * Normalizes a string that represents a file path or URI. | ||
| * | ||
| * This should be called on a string that represents an absolute path to a local file, or a file uri for a local file. | ||
| * Relative paths and files on network shares are not currently supported. | ||
| * | ||
| * We deliberately avoid use of the Path, File and URI libraries as these will make decisions on how paths are handled | ||
| * based on the underlying operating system. This approach provides consistency. | ||
| * | ||
| * @param forceUseForwardSlashes Whether to force the use of forward slashses as path separators, even for files on | ||
| * Windows. Unix systems will always use forward slashes. | ||
| * @return The normalized string. | ||
| */ | ||
| private fun String.toNormalizedFilePath(forceUseForwardSlashes: Boolean): String { | ||
| val fileScheme = "file:" | ||
| val windowsSeparator = "\\" | ||
| val unixSeparator = "/" | ||
|
|
||
| // Strip the scheme and standardise separators on Unix for now. | ||
| val normalizedPath = this.removePrefix(fileScheme).replace(windowsSeparator, unixSeparator) | ||
| var targetSeparator = unixSeparator | ||
|
|
||
| // Split the path into parts, filtering out any blanks or references to the current directory. | ||
| val parts = normalizedPath.split(unixSeparator).filter { it.isNotBlank() && it != "." }.mapIndexed { idx, value -> | ||
| if (idx == 0) { | ||
| // Since we only support local files, we can use the first element of the path can tell us whether we | ||
| // are dealing with a Windows or unix file. | ||
| if (value.startsWithWindowsDriveLetter()) { | ||
| // Change to using Windows separators (if allowed) and capitalize the drive letter. | ||
| if (!forceUseForwardSlashes) targetSeparator = windowsSeparator | ||
| value.uppercase() | ||
| } else { | ||
| // On a Unix system, start with a slash representing root. | ||
| unixSeparator + value | ||
| } | ||
| } else value | ||
| } | ||
|
|
||
| // Removing any references to the parent directory (we have already removed references to the current directory). | ||
| val stack = mutableListOf<String>() | ||
| for (part in parts) { | ||
| if (part == "..") { | ||
| if (stack.isNotEmpty()) stack.removeAt(stack.size - 1) | ||
| } else stack.add(part) | ||
| } | ||
| return stack.joinToString(targetSeparator) | ||
| } | ||
|
|
||
| /** | ||
| * Converts a string representing a file path to a normalised form. @see io.snyk.plugin.UtilsKt.toNormalizedFilePath | ||
| */ | ||
| fun String.toFilePathString(): String { | ||
| return this.toNormalizedFilePath(forceUseForwardSlashes = false) | ||
| return this.path.fromPathToUriString() | ||
| } | ||
|
|
||
| /** | ||
| * Converts a string representing a file path to a normalised form. @see io.snyk.plugin.UtilsKt.toNormalizedFilePath | ||
| */ | ||
| fun String.toFileURIString(): String { | ||
| var pathString = this.toNormalizedFilePath(forceUseForwardSlashes = true) | ||
|
|
||
| // If we are handling a Windows path it may not have a leading slash, so add one. | ||
| if (pathString.startsWithWindowsDriveLetter()) { | ||
| pathString = "/$pathString" | ||
| } | ||
|
|
||
| // Add a file scheme. We use two slashes as standard. | ||
| return "file://$pathString" | ||
| fun String.fromPathToUriString(): String { | ||
| return Paths.get(this).normalize().toUri().toASCIIString() | ||
| } | ||
|
Comment on lines
+447
to
449
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
|
|
||
| private fun String.startsWithWindowsDriveLetter(): Boolean { | ||
|
|
@@ -517,7 +456,7 @@ fun VirtualFile.getDocument(): Document? = runReadAction { FileDocumentManager.g | |
|
|
||
| fun Project.getContentRootPaths(): SortedSet<Path> { | ||
| return getContentRootVirtualFiles() | ||
| .mapNotNull { it.path.toNioPathOrNull() } | ||
| .mapNotNull { it.path.toNioPathOrNull()?.normalize() } | ||
| .toSortedSet() | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,14 +12,15 @@ import com.intellij.openapi.project.ProjectManager | |
| import com.intellij.openapi.util.Disposer | ||
| import com.intellij.openapi.util.io.toNioPathOrNull | ||
| import com.intellij.openapi.vfs.VirtualFile | ||
| import io.snyk.plugin.fromUriToPath | ||
| import io.snyk.plugin.getCliFile | ||
| import io.snyk.plugin.getContentRootVirtualFiles | ||
| import io.snyk.plugin.getSnykTaskQueueService | ||
| import io.snyk.plugin.getWaitForResultsTimeout | ||
| import io.snyk.plugin.pluginSettings | ||
| import io.snyk.plugin.runInBackground | ||
| import io.snyk.plugin.toFilePathString | ||
| import io.snyk.plugin.toLanguageServerURI | ||
| import io.snyk.plugin.ui.SnykBalloonNotificationHelper | ||
| import io.snyk.plugin.ui.toolwindow.SnykPluginDisposable | ||
| import org.eclipse.lsp4j.ClientCapabilities | ||
| import org.eclipse.lsp4j.ClientInfo | ||
|
|
@@ -64,9 +65,9 @@ import snyk.common.lsp.commands.COMMAND_WORKSPACE_FOLDER_SCAN | |
| import snyk.common.lsp.commands.SNYK_GENERATE_ISSUE_DESCRIPTION | ||
| import snyk.common.lsp.progress.ProgressManager | ||
| import snyk.common.lsp.settings.FolderConfigSettings | ||
| import snyk.common.lsp.settings.IssueViewOptions | ||
| import snyk.common.lsp.settings.LanguageServerSettings | ||
| import snyk.common.lsp.settings.SeverityFilter | ||
| import snyk.common.lsp.settings.IssueViewOptions | ||
| import snyk.common.removeTrailingSlashesIfPresent | ||
| import snyk.pluginInfo | ||
| import snyk.trust.WorkspaceTrustService | ||
|
|
@@ -97,7 +98,7 @@ class LanguageServerWrapper( | |
|
|
||
| // internal for test set up | ||
| internal val configuredWorkspaceFolders: MutableSet<WorkspaceFolder> = Collections.synchronizedSet(mutableSetOf()) | ||
| internal var folderConfigsRefreshed: MutableMap<String, Boolean> = ConcurrentHashMap() | ||
| private var folderConfigsRefreshed: MutableMap<String, Boolean> = ConcurrentHashMap() | ||
| private var disposed = false | ||
| get() { | ||
| return ApplicationManager.getApplication().isDisposed || field | ||
|
|
@@ -199,11 +200,11 @@ class LanguageServerWrapper( | |
| LanguageServerRestartListener.getInstance() | ||
| refreshFeatureFlags() | ||
| } else { | ||
| logger.warn("Language Server initialization did not succeed") | ||
| logger.error("Snyk Language Server process launch failed.") | ||
| } | ||
| } catch (e: Exception) { | ||
| logger.warn(e) | ||
| process.destroy() | ||
| logger.error("Initialization of Snyk Language Server failed", e) | ||
| if (processIsAlive()) process.destroyForcibly() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The process destruction should check if the process is initialized before attempting to destroy it forcibly. Consider using |
||
| isInitialized = false | ||
| } | ||
| } | ||
|
|
@@ -217,7 +218,7 @@ class LanguageServerWrapper( | |
| lsp4jLogger.level = Level.OFF | ||
| messageProducerLogger.level = Level.OFF | ||
| try { | ||
| val shouldShutdown = lsIsAlive() | ||
| val shouldShutdown = processIsAlive() | ||
| executorService.submit { | ||
| if (shouldShutdown) { | ||
| val project = ProjectUtil.getActiveProject() | ||
|
|
@@ -233,19 +234,19 @@ class LanguageServerWrapper( | |
| // we don't care | ||
| } finally { | ||
| try { | ||
| if (lsIsAlive()) languageServer.exit() | ||
| if (processIsAlive()) languageServer.exit() | ||
| } catch (ignore: Exception) { | ||
| // do nothing | ||
| } finally { | ||
| if (lsIsAlive()) process.destroyForcibly() | ||
| if (processIsAlive()) process.destroyForcibly() | ||
| } | ||
| lsp4jLogger.level = previousLSP4jLogLevel | ||
| messageProducerLogger.level = previousMessageProducerLevel | ||
| configuredWorkspaceFolders.clear() | ||
| } | ||
| } | ||
|
|
||
| private fun lsIsAlive() = ::process.isInitialized && process.isAlive | ||
| private fun processIsAlive() = ::process.isInitialized && process.isAlive | ||
|
|
||
| fun getWorkspaceFoldersFromRoots(project: Project): Set<WorkspaceFolder> { | ||
| if (disposed || project.isDisposed) return emptySet() | ||
|
|
@@ -336,7 +337,7 @@ class LanguageServerWrapper( | |
| added.filter { !configuredWorkspaceFolders.contains(it) }, | ||
| removed.filter { configuredWorkspaceFolders.contains(it) }, | ||
| ) | ||
| if (params.event.added.size > 0 || params.event.removed.size > 0) { | ||
| if (params.event.added.isNotEmpty() || params.event.removed.isNotEmpty()) { | ||
| languageServer.workspaceService.didChangeWorkspaceFolders(params) | ||
| configuredWorkspaceFolders.removeAll(removed) | ||
| configuredWorkspaceFolders.addAll(added) | ||
|
|
@@ -485,11 +486,12 @@ class LanguageServerWrapper( | |
| // the folderConfigs in language server | ||
| val folderConfigs = configuredWorkspaceFolders | ||
| .filter { | ||
| val folderPath = it.uri.toFilePathString() | ||
| val folderPath = it.uri.fromUriToPath().toString() | ||
| folderConfigsRefreshed[folderPath] == true | ||
| }.map { | ||
| val folderPath = it.uri.toFilePathString() | ||
| service<FolderConfigSettings>().getFolderConfig(folderPath) } | ||
| val folderPath = it.uri.fromUriToPath().toString() | ||
| service<FolderConfigSettings>().getFolderConfig(folderPath) | ||
| } | ||
| .toList() | ||
|
|
||
| return LanguageServerSettings( | ||
|
|
@@ -621,7 +623,13 @@ class LanguageServerWrapper( | |
|
|
||
| fun addContentRoots(project: Project) { | ||
| if (disposed || project.isDisposed) return | ||
| ensureLanguageServerInitialized() | ||
| if (!ensureLanguageServerInitialized()) { | ||
| SnykBalloonNotificationHelper.showWarn( | ||
| "Unable to initialize the Snyk Language Server. The plugin will be non-functional.", | ||
| project | ||
| ) | ||
| return | ||
| } | ||
| ensureLanguageServerProtocolVersion(project) | ||
| updateConfiguration(false) | ||
| val added = getWorkspaceFoldersFromRoots(project) | ||
|
|
@@ -665,7 +673,13 @@ class LanguageServerWrapper( | |
| executeCommand(param) | ||
| } | ||
|
|
||
| fun sendSubmitIgnoreRequestCommand(workflow: String, issueId: String, ignoreType: String, ignoreReason: String, ignoreExpirationDate: String) { | ||
| fun sendSubmitIgnoreRequestCommand( | ||
| workflow: String, | ||
| issueId: String, | ||
| ignoreType: String, | ||
| ignoreReason: String, | ||
| ignoreExpirationDate: String | ||
| ) { | ||
| if (!ensureLanguageServerInitialized()) throw RuntimeException("couldn't initialize language server") | ||
| try { | ||
| val param = ExecuteCommandParams() | ||
|
|
@@ -753,6 +767,15 @@ class LanguageServerWrapper( | |
| shutdown() | ||
| } | ||
|
|
||
| fun getFolderConfigsRefreshed(): Map<String?, Boolean?> { | ||
| return Collections.unmodifiableMap(this.folderConfigsRefreshed) | ||
| } | ||
|
|
||
| fun updateFolderConfigRefresh(folderPath: String, refreshed: Boolean) { | ||
| val path = Paths.get(folderPath).normalize().toAbsolutePath().toString() | ||
| this.folderConfigsRefreshed[path] = refreshed | ||
| } | ||
|
|
||
|
|
||
| companion object { | ||
| private var instance: LanguageServerWrapper? = null | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
fromUriToPath()method doesn't handle potential exceptions fromURI.create(). If an invalid URI string is passed, this will throw anIllegalArgumentException. Consider adding exception handling to gracefully handle malformed URIs.