Skip to content

[SourceKit] Adjust newlines between decls #72553

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Mar 25, 2024
Merged

[SourceKit] Adjust newlines between decls #72553

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions include/swift/AST/PrintOptions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ struct PrintOptions {
bool PrintInSILBody = false;

/// Whether to use an empty line to separate two members in a single decl.
bool EmptyLineBetweenMembers = false;
bool EmptyLineBetweenDecls = false;

/// Whether to print empty members of a declaration on a single line, e.g.:
/// ```
Expand Down Expand Up @@ -692,7 +692,7 @@ struct PrintOptions {
result.SkipUnderscoredStdlibProtocols = true;
result.SkipUnsafeCXXMethods = true;
result.SkipDeinit = true;
result.EmptyLineBetweenMembers = true;
result.EmptyLineBetweenDecls = true;
result.CascadeDocComment = true;
result.ShouldQualifyNestedDeclarations =
QualifyNestedDeclarations::Always;
Expand Down Expand Up @@ -755,7 +755,7 @@ struct PrintOptions {
static PrintOptions printSwiftFileInterface(bool printFullConvention) {
PrintOptions result = printInterface(printFullConvention);
result.AccessFilter = AccessLevel::Internal;
result.EmptyLineBetweenMembers = true;
result.EmptyLineBetweenDecls = true;
return result;
}

Expand Down
17 changes: 2 additions & 15 deletions lib/AST/ASTPrinter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -705,6 +705,7 @@ class PrintAST : public ASTVisitor<PrintAST> {
StringRef RawText =
RC->getRawText(ClangContext.getSourceManager()).rtrim("\n\r");
trimLeadingWhitespaceFromLines(RawText, WhitespaceToTrim, Lines);
indent();
bool FirstLine = true;
for (auto Line : Lines) {
if (FirstLine)
Expand Down Expand Up @@ -1134,20 +1135,6 @@ class PrintAST : public ASTVisitor<PrintAST> {
Printer.setSynthesizedTarget(Options.TransformContext->getDecl());
}

// We want to print a newline before doc comments. Swift code already
// handles this, but we need to insert it for clang doc comments when not
// printing other clang comments. Do it now so the printDeclPre callback
// happens after the newline.
if (Options.PrintDocumentationComments && D->hasClangNode()) {
auto clangNode = D->getClangNode();
auto clangDecl = clangNode.getAsDecl();
if (clangDecl &&
clangDecl->getASTContext().getRawCommentForAnyRedecl(clangDecl)) {
Printer.printNewline();
indent();
}
}

Printer.callPrintDeclPre(D, Options.BracketOptions);
Comment on lines -1137 to 1138
Copy link
Member Author

Choose a reason for hiding this comment

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

This was leftover from generated interface printing normal comments between decls, which was hooking "PrintDeclPre". Not needed.


if (Options.PrintCompatibilityFeatureChecks) {
Expand Down Expand Up @@ -2724,7 +2711,7 @@ void PrintAST::printMembers(ArrayRef<Decl *> members, bool needComma,
if (!member->shouldPrintInContext(Options))
continue;

if (Options.EmptyLineBetweenMembers)
if (Options.EmptyLineBetweenDecls)
Printer.printNewline();
indent();
visit(member);
Expand Down
56 changes: 35 additions & 21 deletions lib/IDE/ModuleInterfacePrinting.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,11 +167,13 @@ static bool printModuleInterfaceDecl(Decl *D,
};
}
}
if (!LeadingComment.empty() && Options.shouldPrint(D))
Printer << LeadingComment << "\n";
if (!LeadingComment.empty() && Options.shouldPrint(D)) {
Printer << LeadingComment;
Printer.printNewline();
}
if (D->print(Printer, Options)) {
if (Options.BracketOptions.shouldCloseNominal(D))
Printer << "\n";
Printer.printNewline();
Copy link
Member Author

Choose a reason for hiding this comment

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

.printNewline() instead of << "\n" because that's ASTPrinter's style.

Options.BracketOptions = BracketOptions();
if (auto NTD = dyn_cast<NominalTypeDecl>(D)) {
std::queue<NominalTypeDecl *> SubDecls{{NTD}};
Expand All @@ -194,11 +196,13 @@ static bool printModuleInterfaceDecl(Decl *D,
}
if (extensionHasClangNode(Ext))
continue; // will be printed in its source location, see above.
Printer << "\n";
if (!LeadingComment.empty())
Printer << LeadingComment << "\n";
Printer.printNewline();
if (!LeadingComment.empty()) {
Printer << LeadingComment;
Printer.printNewline();
}
Ext->print(Printer, Options);
Printer << "\n";
Printer.printNewline();
}
for (auto Sub : Ext->getMembers())
if (auto N = dyn_cast<NominalTypeDecl>(Sub))
Expand Down Expand Up @@ -226,7 +230,7 @@ static bool printModuleInterfaceDecl(Decl *D,
if (ET.IsSynthesized)
Options.clearSynthesizedExtension();
if (Options.BracketOptions.shouldCloseExtension(ET.Ext))
Printer << "\n";
Printer.printNewline();
}
});
}
Expand All @@ -250,9 +254,11 @@ static bool printModuleInterfaceDecl(Decl *D,
ET.Ext, !Opened, Decls.back().Ext == ET.Ext, true
};
if (Options.BracketOptions.shouldOpenExtension(ET.Ext)) {
Printer << "\n";
if (Options.shouldPrint(ET.Ext) && !LeadingComment.empty())
Printer << LeadingComment << "\n";
Printer.printNewline();
if (Options.shouldPrint(ET.Ext) && !LeadingComment.empty()) {
Printer << LeadingComment;
Printer.printNewline();
}
}
if (ET.IsSynthesized) {
if (ET.EnablingExt)
Expand All @@ -265,7 +271,7 @@ static bool printModuleInterfaceDecl(Decl *D,
if (ET.IsSynthesized)
Options.clearSynthesizedExtension();
if (Options.BracketOptions.shouldCloseExtension(ET.Ext)) {
Printer << "\n";
Printer.printNewline();
}
}
});
Expand Down Expand Up @@ -411,18 +417,22 @@ static void printCrossImportOverlays(ModuleDecl *Declaring, ASTContext &Ctx,
BystanderList += Bystanders[I].str();
}

Printer << "\n// MARK: - " << BystanderList << " Additions\n\n";
Printer.printNewline();
Printer << "// MARK: - " << BystanderList << " Additions";
Printer.printNewline();
Printer.printNewline();
for (auto *Import : DeclLists.first)
PrintDecl(Import);
Printer << "\n";
if (!DeclLists.first.empty())
Printer.printNewline();

std::string PerDeclComment = "// Available when " + BystanderList;
PerDeclComment += Bystanders.size() == 1 ? " is" : " are";
PerDeclComment += " imported with " + Declaring->getNameStr().str();

for (auto *D : DeclLists.second) {
if (PrintDecl(D, PerDeclComment))
Printer << "\n";
if (PrintDecl(D, PerDeclComment) && Options.EmptyLineBetweenDecls)
Printer.printNewline();
}
}
}
Expand Down Expand Up @@ -668,7 +678,7 @@ void swift::ide::printModuleInterface(
if (!TargetMod->isStdlibModule()) {
for (auto *D : ImportDecls)
PrintDecl(D);
Printer << "\n";
Printer.printNewline();
}

{
Expand All @@ -686,14 +696,15 @@ void swift::ide::printModuleInterface(

for (auto CM : ClangModules) {
for (auto DeclAndLoc : ClangDecls[CM.first])
PrintDecl(DeclAndLoc.first);
if (PrintDecl(DeclAndLoc.first) && Options.EmptyLineBetweenDecls)
Printer.printNewline();
Comment on lines 697 to +700
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the main change of this PR. Newline after printing a decl just like Swift decls few lines below

}
}

if (!(TraversalOptions & ModuleTraversal::SkipOverlay) || !TargetClangMod) {
for (auto *D : SwiftDecls) {
if (PrintDecl(D))
Printer << "\n";
if (PrintDecl(D) && Options.EmptyLineBetweenDecls)
Printer.printNewline();
Copy link
Member Author

Choose a reason for hiding this comment

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

This makes swift-ide-test -print-interface NOT print newlines between top-level decls.
Currently EmptyLineBetweenDecls (formerly EmptyLineBetweenMembers) is enabled in generated-interface only.

}

// If we're printing the entire target module (not specific sub-groups),
Expand All @@ -704,6 +715,8 @@ void swift::ide::printModuleInterface(
AdjustedOptions, PrintSynthesizedExtensions);
}
}
// Flush pending newlines.
Printer.forceNewlines();
}

static SourceLoc getDeclStartPosition(SourceFile &File) {
Expand Down Expand Up @@ -813,8 +826,9 @@ void swift::ide::printHeaderInterface(
continue;
}
if (D->print(Printer, AdjustedOptions))
Printer << "\n";
Printer.printNewline();
}
Printer.forceNewlines();
}

void swift::ide::printSymbolicSwiftClangModuleInterface(
Expand Down
16 changes: 0 additions & 16 deletions test/IDE/Inputs/foo_swift_module.printed.comments.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,19 @@
import SwiftOnoneSupport

func %%% (lhs: Int, rhs: Int) -> Int

postfix func =-> (lhs: Int) -> Int

postfix func => (lhs: Int) -> Int

struct BarGenericSwiftStruct1<T> {
init(t: T)
func bar1InstanceFunc()
}

struct BarGenericSwiftStruct2<T, U> where T : BarProtocol {
init(t: T, u: U)
func bar2InstanceFunc()
}

protocol BarProtocol {
func instanceFunc()
}

/// FooSwiftStruct Aaa.
/**
* Bbb.
Expand All @@ -36,31 +30,21 @@ struct FooSwiftStruct {
func fooInstanceFunc()
init()
}

/// rdar://18457785
enum MyQuickLookObject {
/// A rectangle.
///
/// Uses explicit coordinates to avoid coupling a particular Cocoa type.
case Rectangle(Float64, Float64, Float64, Float64)
}

var globalVar: Int

func hiddenImport()

func overlayedFoo()

func visibleImport()

precedencegroup High {
associativity: left
higherThan: BitwiseShiftPrecedence
}

infix operator %%% : High

postfix operator =>

postfix operator =->

1 change: 0 additions & 1 deletion test/IDE/Inputs/print_clang_header/header-to-print.h.command-line-include.printed.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ class BaseInHead {
class func doIt(_ arg: Int32)
func doIt(_ arg: Int32)
}

/// Awesome name.
class SameName {
}
Expand Down
1 change: 0 additions & 1 deletion test/IDE/Inputs/print_clang_header/header-to-print.h.module.printed.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ class BaseInHead {
class func doIt(_ arg: Int32)
func doIt(_ arg: Int32)
}

/// Awesome name.
class SameName {
}
Expand Down
1 change: 0 additions & 1 deletion test/IDE/Inputs/print_clang_header/header-to-print.h.printed.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ class BaseInHead {
class func doIt(_ arg: Int32)
func doIt(_ arg: Int32)
}

/// Awesome name.
class SameName {
}
Expand Down
2 changes: 1 addition & 1 deletion test/IDE/print_module_comments.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@
// RUN: %target-swift-frontend -disable-implicit-concurrency-module-import -disable-implicit-string-processing-module-import -emit-module %S/Inputs/foo_swift_module.swift -emit-module-path %t/foo_swift_module.swiftmodule -emit-module-doc-path %t/foo_swift_module.swiftdoc
//
// RUN: %target-swift-ide-test -print-module -source-filename %s -I %t -module-to-print=foo_swift_module > %t.printed.txt
// RUN: diff %t.printed.txt %S/Inputs/foo_swift_module.printed.comments.txt
// RUN: diff -u %S/Inputs/foo_swift_module.printed.comments.txt %t.printed.txt

1 change: 1 addition & 0 deletions test/Interop/Cxx/symbolic-imports/indexing-emit-objcxx-symbolic-module-interface.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ import ObjCxxModule
// CHECK-EMPTY:
// CHECK-NEXT: public static func freeCxxFunction(_ x: Int32, _ y: Int32) -> Int32
// CHECK-NEXT: }
// CHECK-EMPTY:
// CHECK-NEXT: open class ObjCClass : NSObject {
// CHECK-EMPTY:
// CHECK-NEXT: open func myTestMethod()
Expand Down
1 change: 1 addition & 0 deletions test/Interop/Cxx/symbolic-imports/indexing-emit-symbolic-module-interface-used-decls.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,5 @@ public func useConcreteTemplate() {
// CHECK-NEXT: public func methodFunc(_ x: Any)
// CHECK-NEXT:}
// CHECK-NEXT:}
// CHECK-EMPTY:
// CHECK-NEXT: public typealias TemplateRecordInt = ns.TemplateRecord
2 changes: 2 additions & 0 deletions test/Interop/Cxx/symbolic-imports/indexing-emit-symbolic-module-interface.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,5 +158,7 @@ import CxxModule
// CHECK-EMPTY:
// CHECK-NEXT: static func freeFunction(_ x: Int32, _ y: Int32) -> Int32
// CHECK-NEXT: }
// CHECK-EMPTY:
// CHECK-NEXT: typealias MyType = ns.TemplateRecord
// CHECK-EMPTY:
// CHECK-NEXT: typealias MyType2 = TransitiveStruct
12 changes: 6 additions & 6 deletions test/SourceKit/CursorInfo/cursor_generated_interface.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public class ASwiftType {
}

// LibA is a mixed framework with no source info and a submodule
// RUN: %sourcekitd-test -req=interface-gen-open -module LibA -- -F %t/frameworks -target %target-triple == -req=cursor -pos=11:36 -print-raw-response | %FileCheck %s --check-prefix=CHECKA
// RUN: %sourcekitd-test -req=interface-gen-open -module LibA -- -F %t/frameworks -target %target-triple == -req=cursor -pos=12:36 -print-raw-response | %FileCheck %s --check-prefix=CHECKA
// CHECKA: key.name: "ASwiftType"
// CHECKA: key.modulename: "LibA"
// CHECKA: key.decl_lang: source.lang.swift
Expand All @@ -60,7 +60,7 @@ framework module LibA {
@interface AObjcType
@end

// RUN: %sourcekitd-test -req=interface-gen-open -module LibA -- -F %t/frameworks -target %target-triple == -req=cursor -pos=11:54 -print-raw-response | %FileCheck %s --check-prefix=CHECKAOBJ
// RUN: %sourcekitd-test -req=interface-gen-open -module LibA -- -F %t/frameworks -target %target-triple == -req=cursor -pos=12:54 -print-raw-response | %FileCheck %s --check-prefix=CHECKAOBJ
// CHECKAOBJ: key.name: "AObjcType"
// CHECKAOBJ: key.line: [[@LINE-5]]
// CHECKAOBJ: key.column: 12
Expand All @@ -72,7 +72,7 @@ framework module LibA {
@interface ASubType
@end

// RUN: %sourcekitd-test -req=interface-gen-open -module LibA -- -F %t/frameworks -target %target-triple == -req=cursor -pos=11:70 -print-raw-response | %FileCheck %s --check-prefix=CHECKASUB
// RUN: %sourcekitd-test -req=interface-gen-open -module LibA -- -F %t/frameworks -target %target-triple == -req=cursor -pos=12:70 -print-raw-response | %FileCheck %s --check-prefix=CHECKASUB
// CHECKASUB: key.name: "ASubType"
// CHECKASUB: key.line: [[@LINE-5]]
// CHECKASUB: key.column: 12
Expand All @@ -84,7 +84,7 @@ framework module LibA {
public class BType {}

// LibB has source info
// RUN: %sourcekitd-test -req=interface-gen-open -module LibA -- -F %t/frameworks -target %target-triple == -req=cursor -pos=13:32 -print-raw-response | %FileCheck %s --check-prefix=CHECKB
// RUN: %sourcekitd-test -req=interface-gen-open -module LibA -- -F %t/frameworks -target %target-triple == -req=cursor -pos=14:32 -print-raw-response | %FileCheck %s --check-prefix=CHECKB
// CHECKB: key.name: "BType"
// CHECKB: key.line: [[@LINE-5]]
// CHECKB: key.column: 14
Expand All @@ -96,7 +96,7 @@ public class BType {}
public class CType {}

// LibC has no source info
// RUN: %sourcekitd-test -req=interface-gen-open -module LibA -- -F %t/frameworks -target %target-triple == -req=cursor -pos=13:47 -print-raw-response | %FileCheck %s --check-prefix=CHECKC
// RUN: %sourcekitd-test -req=interface-gen-open -module LibA -- -F %t/frameworks -target %target-triple == -req=cursor -pos=14:47 -print-raw-response | %FileCheck %s --check-prefix=CHECKC
// CHECKC: key.name: "CType"
// CHECKC: key.modulename: "LibC"
// CHECKC: key.decl_lang: source.lang.swift
Expand All @@ -105,7 +105,7 @@ public class CType {}
@interface DType
@end

// RUN: %sourcekitd-test -req=interface-gen-open -module LibA -- -F %t/frameworks -target %target-triple == -req=cursor -pos=13:57 -print-raw-response | %FileCheck %s --check-prefix=CHECKD
// RUN: %sourcekitd-test -req=interface-gen-open -module LibA -- -F %t/frameworks -target %target-triple == -req=cursor -pos=14:57 -print-raw-response | %FileCheck %s --check-prefix=CHECKD
// CHECKD: key.name: "DType"
// CHECKD: key.line: [[@LINE-5]]
// CHECKD: key.column: 12
Expand Down
16 changes: 8 additions & 8 deletions test/SourceKit/CursorInfo/cursor_info_async.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ import Foo
// RUN: %sourcekitd-test -req=interface-gen-open -module Foo -- \
// RUN: -F %S/../Inputs/libIDE-mock-sdk \
// RUN: -target %target-triple %clang-importer-sdk-nosource -I %t \
// RUN: == -async -dont-print-request -req=cursor -pos=49:15 \
// RUN: == -async -dont-print-request -req=cursor -pos=49:15 \
// RUN: == -async -dont-print-request -req=cursor -pos=49:15 \
// RUN: == -async -dont-print-request -req=cursor -pos=49:15 \
// RUN: == -async -dont-print-request -req=cursor -pos=49:15 \
// RUN: == -async -dont-print-request -req=cursor -pos=49:15 \
// RUN: == -async -dont-print-request -req=cursor -pos=49:15 \
// RUN: == -async -dont-print-request -req=cursor -pos=49:15 | %FileCheck %s -check-prefix=CHECK-FOO
// RUN: == -async -dont-print-request -req=cursor -pos=54:15 \
// RUN: == -async -dont-print-request -req=cursor -pos=54:15 \
// RUN: == -async -dont-print-request -req=cursor -pos=54:15 \
// RUN: == -async -dont-print-request -req=cursor -pos=54:15 \
// RUN: == -async -dont-print-request -req=cursor -pos=54:15 \
// RUN: == -async -dont-print-request -req=cursor -pos=54:15 \
// RUN: == -async -dont-print-request -req=cursor -pos=54:15 \
// RUN: == -async -dont-print-request -req=cursor -pos=54:15 | %FileCheck %s -check-prefix=CHECK-FOO

// CHECK-FOO: <decl.struct><syntaxtype.keyword>struct</syntaxtype.keyword> <decl.name>FooRuncingOptions
// CHECK-FOO: <decl.struct><syntaxtype.keyword>struct</syntaxtype.keyword> <decl.name>FooRuncingOptions
Expand Down
Loading