Diagnose invalid string segment indentation in multi-line string literals

Author: ahoppen

Sources/SwiftParser/StringLiterals.swift

@@ -162,41 +162,80 @@ extension Parser {
     // -------------------------------------------------------------------------
     // Check open quote followed by newline
 
+    // Condition for the loop below that indicates whether the segment we are
+    // iterating over is on a new line.
+    var isSegmentOnNewLine: Bool
+
     if firstSegment?.content.tokenText == "\n" {
       openQuote = openQuote.extendingTrailingTrivia(by: [.newlines(1)], arena: self.arena)
+      isSegmentOnNewLine = true
     } else {
       if let firstSegment = firstSegment {
         middleSegments.insert(.stringSegment(firstSegment), at: 0)
       }
+      isSegmentOnNewLine = false
       unexpectedBeforeOpenQuote = [openQuote]
       openQuote = RawTokenSyntax(missing: openQuote.tokenKind, trailingTriviaPieces: [.newlines(1)] + indentationTrivia, arena: self.arena)
     }
 
     // -------------------------------------------------------------------------
-    // Check indentation of segments
+    // Check indentation of segments and escaped newlines at end of segment
 
     for (index, segment) in middleSegments.enumerated() {
       switch segment {
       case .stringSegment(var segment):
-        assert(segment.unexpectedBeforeContent == nil, "Segment should not have unexpected before content")
-        assert(segment.content.leadingTriviaByteLength == 0, "Segment should not have leading trivia")
-        if segment.content.tokenText.hasPrefix(indentation) {
-          segment = RawStringSegmentSyntax(
-            content: segment.content.reclassifyAsLeadingTrivia(indentationTrivia, arena: self.arena),
-            arena: self.arena
-          )
-        } else {
-          // TODO: Diagnose
+        if segment.content.isMissing {
+          // Don't diagnose incorrect indentation for segments that we synthesized
+          break
+        }
+        // We are not considering unexpected and leading trivia for indentation
+        // computation. If these assertions are violated, we can probably lift
+        // them but we would need to check the produce the expected results.
+        assert(segment.unexpectedBeforeContent == nil && segment.content.leadingTriviaByteLength == 0)
+
+        // Re-classify indentation as leading trivia
+        if isSegmentOnNewLine {
+          if segment.content.tokenText.hasPrefix(indentation) {
+            segment = RawStringSegmentSyntax(
+              segment.unexpectedBeforeContent,
+              content: segment.content.reclassifyAsLeadingTrivia(indentationTrivia, arena: self.arena),
+              segment.unexpectedAfterContent,
+              arena: self.arena
+            )
+          } else {
+            let actualIndentation = segment.content.tokenText.prefix(while: { $0 == UInt8(ascii: " ") || $0 == UInt8(ascii: "\t") })
+            let actualIndentationTriva = TriviaParser.parseTrivia(SyntaxText(rebasing: actualIndentation), position: .leading)
+            let content = segment.content.reclassifyAsLeadingTrivia(
+              actualIndentationTriva,
+              lexerError: LexerError(.insufficientIndentationInMultilineStringLiteral, byteOffset: 0),
+              arena: self.arena
+            )
+            segment = RawStringSegmentSyntax(
+              segment.unexpectedBeforeContent,
+              content: content,
+              segment.unexpectedAfterContent,
+              arena: self.arena
+            )
+          }
         }
+
+        isSegmentOnNewLine = segment.content.tokenText.hasSuffix("\n")
+
+        // If the segment has a `\` in front of its trailing newline, that newline
+        // is not part of the reprsented string and should be trivia.
+
         if segment.content.tokenText.hasSuffix("\\\n") {
           // TODO: Add a backslash trivia kind
           segment = RawStringSegmentSyntax(
+            segment.unexpectedBeforeContent,
             content: segment.content.reclassifyAsTrailingTrivia([.unexpectedText("\\"), .newlines(1)], arena: self.arena),
+            segment.unexpectedAfterContent,
             arena: self.arena
           )
         }
         middleSegments[index] = .stringSegment(segment)
-      case .expressionSegment:
+      case .expressionSegment(let segment):
+        isSegmentOnNewLine = segment.rightParen.trailingTriviaPieces.contains(where: { $0.isNewline })
         // TODO: Check indentation
         break
       }

Sources/SwiftParserDiagnostics/CMakeLists.txt

@@ -11,6 +11,7 @@ add_swift_host_library(SwiftParserDiagnostics
   LexerDiagnosticMessages.swift
   MissingNodesError.swift
   MissingTokenError.swift
+  MultiLineStringLiteralDiagnoticsGenerator.swift
   ParserDiagnosticMessages.swift
   ParseDiagnosticsGenerator.swift
   PresenceUtils.swift

Sources/SwiftParserDiagnostics/LexerDiagnosticMessages.swift

@@ -110,6 +110,11 @@ public extension SwiftSyntax.LexerError {
       return StaticLexerError.expectedBinaryExponentInHexFloatLiteral
     case .expectedDigitInFloatLiteral:
       return StaticLexerError.expectedDigitInFloatLiteral
+    case .insufficientIndentationInMultilineStringLiteral:
+      // This should be diagnosed when visiting the `StringLiteralExprSyntax`
+      // inside `ParseDiagnosticsGenerator` but fall back to an error message
+      // here in case the error is not diagnosed.
+      return InvalidIndentationInMultiLineStringLiteralError(kind: .insufficientIdentation, lines: 1)
     case .invalidBinaryDigitInIntegerLiteral:
       return InvalidDigitInIntegerLiteral(kind: .binary(scalarAtErrorOffset))
     case .invalidDecimalDigitInIntegerLiteral:

Sources/SwiftParserDiagnostics/MultiLineStringLiteralDiagnoticsGenerator.swift

@@ -0,0 +1,147 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2022 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+import SwiftDiagnostics
+import SwiftSyntax
+
+/// A diagnostic that `MultiLineStringLiteralIndentatinDiagnosticsGenerator` is building.
+/// As indentation errors are found on more lines, this diagnostic is modified
+/// to include more fixIts.
+private struct InProgressDiagnostic {
+  let anchor: TokenSyntax
+  let position: AbsolutePosition
+  let kind: InvalidIndentationInMultiLineStringLiteralError.Kind
+  var lines: Int
+  var changes: [FixIt.Change]
+  var handledNodes: [SyntaxIdentifier]
+}
+
+final class MultiLineStringLiteralIndentatinDiagnosticsGenerator: SyntaxVisitor {
+
+  // MARK: Entry
+
+  public static func diagnose(_ node: StringLiteralExprSyntax) -> [(diagnostic: Diagnostic, handledNodes: [SyntaxIdentifier])] {
+    let visitor = MultiLineStringLiteralIndentatinDiagnosticsGenerator(closeQuote: node.closeQuote)
+    visitor.walk(node)
+    visitor.finishInProgressDiagnostic()
+    return visitor.finishedDiagnostics
+  }
+
+  // MARK: Implemenatation
+
+  private let closeQuote: TokenSyntax
+
+  /// Diagnostics that we have finisehed because their incorrect indentation was followed by correct indentation
+  private var finishedDiagnostics: [(diagnostic: Diagnostic, handledNodes: [SyntaxIdentifier])] = []
+
+  /// The diagnostic we are currently building up
+  private var inProgressDiagnostic: InProgressDiagnostic?
+
+  private init(closeQuote: TokenSyntax) {
+    self.closeQuote = closeQuote
+    super.init(viewMode: .sourceAccurate)
+  }
+
+  private func addIncorrectlyIndentedToken(token: TokenSyntax) {
+    // Determine kind and position of the diagnonstic
+    var kind: InvalidIndentationInMultiLineStringLiteralError.Kind = .insufficientIdentation
+    var position = token.positionAfterSkippingLeadingTrivia
+    var positionOffset = 0
+    for (invalidTriviaPiece, missingTriviaPiece) in zip(token.leadingTrivia.decomposed, closeQuote.leadingTrivia.decomposed) {
+      if invalidTriviaPiece == missingTriviaPiece {
+        positionOffset += invalidTriviaPiece.sourceLength.utf8Length
+        continue
+      }
+      switch invalidTriviaPiece {
+      case .tabs: kind = .unexpectedTab
+      case .spaces: kind = .unexpectedSpace
+      default: break
+      }
+      position = token.position.advanced(by: positionOffset)
+      break
+    }
+
+    // If the diagnostic we are currently building has a different kind, we
+    // cannot merge them. Commit the current diagnostic so we can create a new one.
+    if inProgressDiagnostic?.kind != kind {
+      finishInProgressDiagnostic()
+    }
+
+    // Append hte inProgress diagnostic or create a new one.
+    let changes = [FixIt.Change.replaceLeadingTrivia(token: token, newTrivia: closeQuote.leadingTrivia)]
+    let handledNodes = [token.id]
+    if self.inProgressDiagnostic != nil {
+      self.inProgressDiagnostic!.lines += 1
+      self.inProgressDiagnostic!.changes += changes
+      self.inProgressDiagnostic!.handledNodes += handledNodes
+    } else {
+      self.inProgressDiagnostic = InProgressDiagnostic(
+        anchor: token,
+        position: position,
+        kind: kind,
+        lines: 1,
+        changes: changes,
+        handledNodes: handledNodes
+      )
+    }
+  }
+
+  // Finish the diagnostic that's currently in progress so any new indentation
+  // issue that's found will start a new diagnostic.
+  private func finishInProgressDiagnostic() {
+    guard let currentDiagnostic = self.inProgressDiagnostic else {
+      return
+    }
+
+    let diagnostic = Diagnostic(
+      node: Syntax(currentDiagnostic.anchor),
+      position: currentDiagnostic.position,
+      message: InvalidIndentationInMultiLineStringLiteralError(kind: currentDiagnostic.kind, lines: currentDiagnostic.lines),
+      highlights: [],
+      notes: [Note(node: Syntax(closeQuote), message: .shouldMatchIndentationOfClosingQuote)],
+      fixIts: [FixIt(message: .changeIndentationToMatchClosingDelimiter, changes: FixIt.Changes(changes: currentDiagnostic.changes))]
+    )
+
+    finishedDiagnostics.append((diagnostic, currentDiagnostic.handledNodes))
+    self.inProgressDiagnostic = nil
+  }
+
+  private func isOnNewline(_ token: TokenSyntax) -> Bool {
+    if token.leadingTrivia.contains(where: { $0.isNewline }) {
+      return true
+    }
+    guard let previousToken = token.previousToken(viewMode: .sourceAccurate) else {
+      return false
+    }
+    switch previousToken.tokenKind {
+    case .stringSegment(let stringSegment):
+      return stringSegment.hasSuffix("\r") || stringSegment.hasSuffix("\n")
+    default:
+      // FIXME: newlines should never be part of trailing trivia
+      return previousToken.trailingTrivia.contains(where: { $0.isNewline })
+    }
+  }
+
+  override func visit(_ token: TokenSyntax) -> SyntaxVisitorContinueKind {
+    guard isOnNewline(token) else {
+      // We are only interested in tokens at the start of a line
+      return .visitChildren
+    }
+
+    if token.lexerError?.kind == .insufficientIndentationInMultilineStringLiteral {
+      addIncorrectlyIndentedToken(token: token)
+    } else {
+      finishInProgressDiagnostic()
+    }
+    return .visitChildren
+  }
+}

Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift

@@ -80,12 +80,21 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
     notes: [Note] = [],
     fixIts: [FixIt] = [],
     handledNodes: [SyntaxIdentifier] = []
+  ) {
+    let diagnostic = Diagnostic(node: Syntax(node), position: position, message: message, highlights: highlights, notes: notes, fixIts: fixIts)
+    addDiagnostic(diagnostic, handledNodes: handledNodes)
+  }
+
+  /// Produce a diagnostic.
+  func addDiagnostic(
+    _ diagnostic: Diagnostic,
+    handledNodes: [SyntaxIdentifier] = []
   ) {
     if suppressRemainingDiagnostics {
       return
     }
     diagnostics.removeAll(where: { handledNodes.contains($0.node.id) })
-    diagnostics.append(Diagnostic(node: Syntax(node), position: position, message: message, highlights: highlights, notes: notes, fixIts: fixIts))
+    diagnostics.append(diagnostic)
     self.handledNodes.append(contentsOf: handledNodes)
   }
 
@@ -800,6 +809,16 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
     return .visitChildren
   }
 
+  public override func visit(_ node: StringLiteralExprSyntax) -> SyntaxVisitorContinueKind {
+    if shouldSkip(node) {
+      return .skipChildren
+    }
+    for (diagnostic, handledNodes) in MultiLineStringLiteralIndentatinDiagnosticsGenerator.diagnose(node) {
+      addDiagnostic(diagnostic, handledNodes: handledNodes)
+    }
+    return .visitChildren
+  }
+
   public override func visit(_ node: SubscriptDeclSyntax) -> SyntaxVisitorContinueKind {
     if shouldSkip(node) {
       return .skipChildren

Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift

@@ -273,6 +273,33 @@ public struct InvalidIdentifierError: ParserError {
   }
 }
 
+public struct InvalidIndentationInMultiLineStringLiteralError: ParserError {
+  public enum Kind {
+    case insufficientIdentation
+    case unexpectedSpace
+    case unexpectedTab
+
+    var message: String {
+      switch self {
+      case .insufficientIdentation: return "insufficient indentation"
+      case .unexpectedSpace: return "unexpected space in indentation"
+      case .unexpectedTab: return "unexpected tab in indentation"
+      }
+    }
+  }
+
+  public let kind: Kind
+  public let lines: Int
+
+  public var message: String {
+    if lines == 1 {
+      return "\(kind.message) of line in multi-line string literal"
+    } else {
+      return "\(kind.message) of next \(lines) lines in multi-line string literal"
+    }
+  }
+}
+
 public struct MissingAttributeArgument: ParserError {
   /// The name of the attribute that's missing the argument, without `@`.
   public let attributeName: TypeSyntax
@@ -364,6 +391,31 @@ public struct EffectSpecifierDeclaredHere: ParserNote {
   }
 }
 
+/// A parser fix-it with a static message.
+public struct StaticParserNote: NoteMessage {
+  public let message: String
+  private let messageID: String
+
+  /// This should only be called within a static var on FixItMessage, such
+  /// as the examples below. This allows us to pick up the messageID from the
+  /// var name.
+  fileprivate init(_ message: String, messageID: String = #function) {
+    self.message = message
+    self.messageID = messageID
+  }
+
+  public var fixItID: MessageID {
+    MessageID(domain: diagnosticDomain, id: "\(type(of: self)).\(messageID)")
+  }
+}
+
+extension NoteMessage where Self == StaticParserNote {
+  /// Please order alphabetically by property name.
+  public static var shouldMatchIndentationOfClosingQuote: Self {
+    .init("should match indentation here"
+  }
+}
+
 // MARK: - Fix-Its (please sort alphabetically)
 
 /// A parser fix-it with a static message.
@@ -386,6 +438,9 @@ public struct StaticParserFixIt: FixItMessage {
 
 extension FixItMessage where Self == StaticParserFixIt {
   /// Please order alphabetically by property name.
+  public static var changeIndentationToMatchClosingDelimiter: Self {
+    .init("change indentation of this line to match closing delimiter")
+  }
   public static var insertSemicolon: Self {
     .init("insert ';'")
   }

Sources/SwiftSyntax/LexerError.swift

@@ -19,6 +19,7 @@ public struct LexerError: Hashable {
 
     case expectedBinaryExponentInHexFloatLiteral
     case expectedDigitInFloatLiteral
+    case insufficientIndentationInMultilineStringLiteral
     case invalidBinaryDigitInIntegerLiteral
     case invalidDecimalDigitInIntegerLiteral
     case invalidFloatingPointCharacter