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

Rename note to notification throughout the codebase #893

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
34 changes: 17 additions & 17 deletions Sources/LSPTestSupport/TestJSONRPCConnection.swift
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,11 @@ public final class TestClient: MessageHandler {
public var allowUnexpectedNotification: Bool = true

public func appendOneShotNotificationHandler<N>(_ handler: @escaping (Notification<N>) -> Void) {
oneShotNotificationHandlers.append({ anyNote in
guard let note = anyNote as? Notification<N> else {
fatalError("received notification of the wrong type \(anyNote); expected \(N.self)")
oneShotNotificationHandlers.append({ anyNotification in
guard let notification = anyNotification as? Notification<N> else {
fatalError("received notification of the wrong type \(anyNotification); expected \(N.self)")
}
handler(note)
handler(notification)
})
}

Expand Down Expand Up @@ -171,15 +171,15 @@ extension TestClient: Connection {

/// Send a notification and expect a notification in reply synchronously.
/// For testing notifications that behave like requests - e.g. didChange & publishDiagnostics.
public func sendNoteSync<NReply>(
public func sendnotificationSync<NReply>(
issamarabi marked this conversation as resolved.
Show resolved Hide resolved
_ notification: some NotificationType,
_ handler: @escaping (Notification<NReply>) -> Void
) {

let expectation = XCTestExpectation(description: "sendNoteSync - note received")
let expectation = XCTestExpectation(description: "sendnotificationSync - notification received")
issamarabi marked this conversation as resolved.
Show resolved Hide resolved

handleNextNotification { (note: Notification<NReply>) in
handler(note)
handleNextNotification { (notification: Notification<NReply>) in
handler(notification)
expectation.fulfill()
}

Expand All @@ -194,21 +194,21 @@ extension TestClient: Connection {

/// Send a notification and expect two notifications in reply synchronously.
/// For testing notifications that behave like requests - e.g. didChange & publishDiagnostics.
public func sendNoteSync<NSend, NReply1, NReply2>(
public func sendnotificationSync<NSend, NReply1, NReply2>(
issamarabi marked this conversation as resolved.
Show resolved Hide resolved
_ notification: NSend,
_ handler1: @escaping (Notification<NReply1>) -> Void,
_ handler2: @escaping (Notification<NReply2>) -> Void
) where NSend: NotificationType {

let expectation = XCTestExpectation(description: "sendNoteSync - note received")
let expectation = XCTestExpectation(description: "sendnotificationSync - notification received")
issamarabi marked this conversation as resolved.
Show resolved Hide resolved
expectation.expectedFulfillmentCount = 2

handleNextNotification { (note: Notification<NReply1>) in
handler1(note)
handleNextNotification { (notification: Notification<NReply1>) in
handler1(notification)
expectation.fulfill()
}
appendOneShotNotificationHandler { (note: Notification<NReply2>) in
handler2(note)
appendOneShotNotificationHandler { (notification: Notification<NReply2>) in
handler2(notification)
expectation.fulfill()
}

Expand All @@ -230,9 +230,9 @@ public final class TestServer: MessageHandler {
}

public func handle<N: NotificationType>(_ params: N, from clientID: ObjectIdentifier) {
let note = Notification(params, clientID: clientID)
let notification = Notification(params, clientID: clientID)
if params is EchoNotification {
self.client.send(note.params)
self.client.send(notification.params)
} else {
fatalError("Unhandled notification")
}
Expand Down Expand Up @@ -312,7 +312,7 @@ public struct EchoError: RequestType {
}

public struct EchoNotification: NotificationType {
public static var method: String = "test_server/echo_note"
public static var method: String = "test_server/echo_notification"

public var string: String

Expand Down
38 changes: 19 additions & 19 deletions Sources/SourceKitLSP/Clang/ClangLanguageServer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -366,8 +366,8 @@ extension ClangLanguageServerShim {

/// Intercept clangd's `PublishDiagnosticsNotification` to withold it if we're using fallback
/// build settings.
func publishDiagnostics(_ note: Notification<PublishDiagnosticsNotification>) async {
let params = note.params
func publishDiagnostics(_ notification: Notification<PublishDiagnosticsNotification>) async {
let params = notification.params
// Technically, the publish diagnostics notification could still originate
// from when we opened the file with fallback build settings and we could
// have received real build settings since, which haven't been acknowledged
Expand All @@ -394,7 +394,7 @@ extension ClangLanguageServerShim {
)
)
} else {
await sourceKitServer.sendNotificationToClient(note.params)
await sourceKitServer.sendNotificationToClient(notification.params)
}
}

Expand Down Expand Up @@ -431,30 +431,30 @@ extension ClangLanguageServerShim {

// MARK: - Text synchronization

public func openDocument(_ note: DidOpenTextDocumentNotification) async {
openDocuments[note.textDocument.uri] = note.textDocument.language
public func openDocument(_ notification: DidOpenTextDocumentNotification) async {
openDocuments[notification.textDocument.uri] = notification.textDocument.language
// Send clangd the build settings for the new file. We need to do this before
// sending the open notification, so that the initial diagnostics already
// have build settings.
await documentUpdatedBuildSettings(note.textDocument.uri)
clangd.send(note)
await documentUpdatedBuildSettings(notification.textDocument.uri)
clangd.send(notification)
}

public func closeDocument(_ note: DidCloseTextDocumentNotification) {
openDocuments[note.textDocument.uri] = nil
clangd.send(note)
public func closeDocument(_ notification: DidCloseTextDocumentNotification) {
openDocuments[notification.textDocument.uri] = nil
clangd.send(notification)
}

public func changeDocument(_ note: DidChangeTextDocumentNotification) {
clangd.send(note)
public func changeDocument(_ notification: DidChangeTextDocumentNotification) {
clangd.send(notification)
}

public func willSaveDocument(_ note: WillSaveTextDocumentNotification) {
public func willSaveDocument(_ notification: WillSaveTextDocumentNotification) {

}

public func didSaveDocument(_ note: DidSaveTextDocumentNotification) {
clangd.send(note)
public func didSaveDocument(_ notification: DidSaveTextDocumentNotification) {
clangd.send(notification)
}

// MARK: - Build System Integration
Expand All @@ -476,26 +476,26 @@ extension ClangLanguageServerShim {
if let compileCommand = clangBuildSettings?.compileCommand,
let pathString = (try? AbsolutePath(validating: url.path))?.pathString
{
let note = DidChangeConfigurationNotification(
let notification = DidChangeConfigurationNotification(
settings: .clangd(
ClangWorkspaceSettings(
compilationDatabaseChanges: [pathString: compileCommand])
)
)
clangd.send(note)
clangd.send(notification)
}
}

public func documentDependenciesUpdated(_ uri: DocumentURI) {
// In order to tell clangd to reload an AST, we send it an empty `didChangeTextDocument`
// with `forceRebuild` set in case any missing header files have been added.
// This works well for us as the moment since clangd ignores the document version.
let note = DidChangeTextDocumentNotification(
let notification = DidChangeTextDocumentNotification(
textDocument: VersionedTextDocumentIdentifier(uri, version: 0),
contentChanges: [],
forceRebuild: true
)
clangd.send(note)
clangd.send(notification)
}

// MARK: - Text Document
Expand Down
16 changes: 8 additions & 8 deletions Sources/SourceKitLSP/DocumentManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -200,32 +200,32 @@ extension DocumentManager {

/// Convenience wrapper for `open(_:language:version:text:)` that logs on failure.
@discardableResult
func open(_ note: DidOpenTextDocumentNotification) -> DocumentSnapshot? {
let doc = note.textDocument
func open(_ notification: DidOpenTextDocumentNotification) -> DocumentSnapshot? {
let doc = notification.textDocument
return orLog("failed to open document", level: .error) {
try open(doc.uri, language: doc.language, version: doc.version, text: doc.text)
}
}

/// Convenience wrapper for `close(_:)` that logs on failure.
func close(_ note: DidCloseTextDocumentNotification) {
func close(_ notification: DidCloseTextDocumentNotification) {
orLog("failed to close document", level: .error) {
try close(note.textDocument.uri)
try close(notification.textDocument.uri)
}
}

/// Convenience wrapper for `edit(_:newVersion:edits:willEditDocument:updateDocumentTokens:)`
/// that logs on failure.
@discardableResult
func edit(
_ note: DidChangeTextDocumentNotification,
_ notification: DidChangeTextDocumentNotification,
willEditDocument: ((_ before: LineTable, TextDocumentContentChangeEvent) -> Void)? = nil
) -> (preEditSnapshot: DocumentSnapshot, postEditSnapshot: DocumentSnapshot)? {
return orLog("failed to edit document", level: .error) {
return try edit(
note.textDocument.uri,
newVersion: note.textDocument.version,
edits: note.contentChanges,
notification.textDocument.uri,
newVersion: notification.textDocument.version,
edits: notification.contentChanges,
willEditDocument: willEditDocument
)
}
Expand Down
20 changes: 10 additions & 10 deletions Sources/SourceKitLSP/SourceKitServer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1033,12 +1033,12 @@ extension SourceKitServer {
await openDocument(notification, workspace: workspace)
}

private func openDocument(_ note: DidOpenTextDocumentNotification, workspace: Workspace) async {
private func openDocument(_ notification: DidOpenTextDocumentNotification, workspace: Workspace) async {
// Immediately open the document even if the build system isn't ready. This is important since
// we check that the document is open when we receive messages from the build system.
documentManager.open(note)
documentManager.open(notification)

let textDocument = note.textDocument
let textDocument = notification.textDocument
let uri = textDocument.uri
let language = textDocument.language

Expand All @@ -1050,7 +1050,7 @@ extension SourceKitServer {
await workspace.buildSystemManager.registerForChangeNotifications(for: uri, language: language)

// If the document is ready, we can immediately send the notification.
await service.openDocument(note)
await service.openDocument(notification)
}

func closeDocument(_ notification: DidCloseTextDocumentNotification) async {
Expand All @@ -1062,16 +1062,16 @@ extension SourceKitServer {
await self.closeDocument(notification, workspace: workspace)
}

func closeDocument(_ note: DidCloseTextDocumentNotification, workspace: Workspace) async {
func closeDocument(_ notification: DidCloseTextDocumentNotification, workspace: Workspace) async {
// Immediately close the document. We need to be sure to clear our pending work queue in case
// the build system still isn't ready.
documentManager.close(note)
documentManager.close(notification)

let uri = note.textDocument.uri
let uri = notification.textDocument.uri

await workspace.buildSystemManager.unregisterForChangeNotifications(for: uri)

await workspace.documentService[uri]?.closeDocument(note)
await workspace.documentService[uri]?.closeDocument(notification)
}

func changeDocument(_ notification: DidChangeTextDocumentNotification) async {
Expand All @@ -1098,10 +1098,10 @@ extension SourceKitServer {
}

func didSaveDocument(
_ note: DidSaveTextDocumentNotification,
_ notification: DidSaveTextDocumentNotification,
languageService: ToolchainLanguageServer
) async {
await languageService.didSaveDocument(note)
await languageService.didSaveDocument(notification)
}

func didChangeWorkspaceFolders(_ notification: DidChangeWorkspaceFoldersNotification) async {
Expand Down
46 changes: 23 additions & 23 deletions Sources/SourceKitLSP/Swift/Diagnostic.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ extension CodeAction {

/// Creates a CodeAction from a list for sourcekit fixits.
///
/// If this is from a note, the note's description should be passed as `fromNote`.
init?(fixits: SKDResponseArray, in snapshot: DocumentSnapshot, fromNote: String?) {
/// If this is from a notification, the notification's description should be passed as `fromNotification`.
init?(fixits: SKDResponseArray, in snapshot: DocumentSnapshot, fromNotification: String?) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This file is about diagnostics, which do indeed have notes, not notifications. Could you change it back?

Copy link
Author

Choose a reason for hiding this comment

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

I changed these back. Let me know if this works!

var edits: [TextEdit] = []
let editsMapped = fixits.forEach { (_, skfixit) -> Bool in
if let edit = TextEdit(fixit: skfixit, in: snapshot) {
Expand All @@ -40,8 +40,8 @@ extension CodeAction {
}

let title: String
if let fromNote = fromNote {
title = fromNote
if let fromNotification = fromNotification {
title = fromNotification
} else {
guard let startIndex = snapshot.index(of: edits[0].range.lowerBound),
let endIndex = snapshot.index(of: edits[0].range.upperBound),
Expand Down Expand Up @@ -123,7 +123,7 @@ extension Diagnostic {
init?(
_ diag: SKDResponseDictionary,
in snapshot: DocumentSnapshot,
useEducationalNoteAsCode: Bool
useEducationalNotificationAsCode: Bool
) {
// FIXME: this assumes that the diagnostics are all in the same file.

Expand Down Expand Up @@ -175,14 +175,14 @@ extension Diagnostic {

var code: DiagnosticCode? = nil
var codeDescription: CodeDescription? = nil
// If this diagnostic has one or more educational notes, the first one is
// If this diagnostic has one or more educational notifications, the first one is
// treated as primary and used to fill in the diagnostic code and
// description. `useEducationalNoteAsCode` ensures a note name is only used
// description. `useEducationalNotificationAsCode` ensures a notification name is only used
// as a code if the cline supports an extended code description.
if useEducationalNoteAsCode,
let educationalNotePaths: SKDResponseArray = diag[keys.educational_note_paths],
educationalNotePaths.count > 0,
let primaryPath = educationalNotePaths[0]
if useEducationalNotificationAsCode,
let educationalNotificationPaths: SKDResponseArray = diag[keys.educational_notification_paths],
educationalNotificationPaths.count > 0,
let primaryPath = educationalNotificationPaths[0]
{
let url = URL(fileURLWithPath: primaryPath)
let name = url.deletingPathExtension().lastPathComponent
Expand All @@ -192,17 +192,17 @@ extension Diagnostic {

var actions: [CodeAction]? = nil
if let skfixits: SKDResponseArray = diag[keys.fixits],
let action = CodeAction(fixits: skfixits, in: snapshot, fromNote: nil)
let action = CodeAction(fixits: skfixits, in: snapshot, fromNotification: nil)
{
actions = [action]
}

var notes: [DiagnosticRelatedInformation]? = nil
if let sknotes: SKDResponseArray = diag[keys.diagnostics] {
notes = []
sknotes.forEach { (_, sknote) -> Bool in
guard let note = DiagnosticRelatedInformation(sknote, in: snapshot) else { return true }
notes?.append(note)
var notifications: [DiagnosticRelatedInformation]? = nil
if let sknotifications: SKDResponseArray = diag[keys.diagnostics] {
notifications = []
sknotifications.forEach { (_, sknotification) -> Bool in
guard let notification = DiagnosticRelatedInformation(sknotification, in: snapshot) else { return true }
notifications?.append(notification)
return true
}
}
Expand Down Expand Up @@ -230,15 +230,15 @@ extension Diagnostic {
source: "sourcekitd",
message: message,
tags: tags,
relatedInformation: notes,
relatedInformation: notifications,
codeActions: actions
)
}
}

extension DiagnosticRelatedInformation {

/// Creates related information from a sourcekitd note response dictionary.
/// Creates related information from a sourcekitd notification response dictionary.
init?(_ diag: SKDResponseDictionary, in snapshot: DocumentSnapshot) {
let keys = diag.sourcekitd.keys

Expand All @@ -260,7 +260,7 @@ extension DiagnosticRelatedInformation {

var actions: [CodeAction]? = nil
if let skfixits: SKDResponseArray = diag[keys.fixits],
let action = CodeAction(fixits: skfixits, in: snapshot, fromNote: message)
let action = CodeAction(fixits: skfixits, in: snapshot, fromNotification: message)
{
actions = [action]
}
Expand Down Expand Up @@ -297,14 +297,14 @@ extension CachedDiagnostic {
init?(
_ diag: SKDResponseDictionary,
in snapshot: DocumentSnapshot,
useEducationalNoteAsCode: Bool
useEducationalNotificationAsCode: Bool
) {
let sk = diag.sourcekitd
guard
let diagnostic = Diagnostic(
diag,
in: snapshot,
useEducationalNoteAsCode: useEducationalNoteAsCode
useEducationalNotificationAsCode: useEducationalNotificationAsCode
)
else {
return nil
Expand Down