Skip to content

[Diagnostics] Introduce "Educational Notes" for diagnostics #28052

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

Merged
merged 2 commits into from
Nov 8, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1129,6 +1129,8 @@ endif()

add_subdirectory(utils)

add_subdirectory(userdocs)

if ("${CMAKE_SYSTEM_NAME}" STREQUAL "Darwin")
if(SWIFT_BUILD_PERF_TESTSUITE)
add_subdirectory(benchmark)
Expand Down
19 changes: 19 additions & 0 deletions docs/Diagnostics.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,25 @@ The Swift compiler has a setting (under LangOptions) called `DiagnosticsEditorMo

Most diagnostics have no reason to change behavior under editor mode. An example of an exception is the "protocol requirements not satisfied diagnostic"; on the command line, it may be better to show all unsatisfied requirements, while in an IDE a single multi-line fix-it would be preferred.

### Educational Notes ###

**Note**: This feature is currently experimental. It can be enabled by passing the `-Xfrontend -enable-descriptive-diagnostics` flag.

Educational notes are small snippets of documentation attached to a diagnostic which explain relevant language concepts. They are intended to further Swift's goal of progressive disclosure by providing a learning resource at the point of use for users encountering a new error message for the first time. In very limited circumstances, they also allow the main diagnostic message to use more precise and correct terminology (e.g. nominal types) which would otherwise be too unfriendly for beginners.

When outputting diagnostics on the command line, educational notes will be printed after the main diagnostic body if descriptive diagnostics are enabled. When presented in an IDE, it's expected they will be collapsed under a disclosure arrow, info button, or similar to avoid cluttering output.

Generally speaking, a diagnostic should try to provide educational notes for any concepts/terminology which is difficult to understand from context or is especially subtle. Educational notes should:
- Explain a single language concept. This makes them easy to reuse across diagnostics and helps keep them clear, concise, and easy to understand.
- Be written in unabbreviated English. These are longer form messages compared to the main diagnostic, so there is no need to omit needless words and punctuation.
- Not generally exceed 3-4 paragraphs. Educational notes should be clear and easily digestible. Messages which are too long also have the potential to create diagnostics UX issues in some contexts.
- Be accessible. Educational notes should be beginner friendly and avoid assuming unnecesary prior knowledge. The goal is not only to help users understand what a diagnostic is telling them, but also to turn errors and warnings into "teachable moments".
- Include references to relevant chapters of _The Swift Programming Language_ if applicable.
- Be written in Markdown, but avoid excessive markup to avoid impacting the terminal UX.

To add a new educational note:
1. Add a new Markdown file in the `userdocs/diagnostics/` directory containing the contents of the note.
2. Associate the note with one or more diagnostics in EducationalNotes.def.

### Format Specifiers ###

Expand Down
3 changes: 3 additions & 0 deletions include/swift/AST/DiagnosticConsumer.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ struct DiagnosticInfo {
/// DiagnosticInfo of notes which are children of this diagnostic, if any
ArrayRef<DiagnosticInfo *> ChildDiagnosticInfo;

/// Paths to "educational note" diagnostic documentation in the toolchain.
ArrayRef<std::string> EducationalNotePaths;

/// Represents a fix-it, a replacement of one range of text with another.
class FixIt {
CharSourceRange Range;
Expand Down
10 changes: 10 additions & 0 deletions include/swift/AST/DiagnosticEngine.h
Original file line number Diff line number Diff line change
Expand Up @@ -673,6 +673,9 @@ namespace swift {
/// Use descriptive diagnostic style when available.
bool useDescriptiveDiagnostics = false;

/// Path to diagnostic documentation directory.
std::string diagnosticDocumentationPath = "";

friend class InFlightDiagnostic;
friend class DiagnosticTransaction;
friend class CompoundDiagnosticTransaction;
Expand Down Expand Up @@ -723,6 +726,13 @@ namespace swift {
return useDescriptiveDiagnostics;
}

void setDiagnosticDocumentationPath(std::string path) {
diagnosticDocumentationPath = path;
}
StringRef getDiagnosticDocumentationPath() {
return diagnosticDocumentationPath;
}

void ignoreDiagnostic(DiagID id) {
state.setDiagnosticBehavior(id, DiagnosticState::Behavior::Ignore);
}
Expand Down
29 changes: 29 additions & 0 deletions include/swift/AST/EducationalNotes.def
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
//===-- EducationalNotes.def - Diagnostic Documentation Content -*- C++ -*-===//
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2014 - 2019 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
//
//===----------------------------------------------------------------------===//
//
// This file associates diagnostics with relevant educational notes which
// explain important concepts.
//
//===----------------------------------------------------------------------===//

#ifndef EDUCATIONAL_NOTES
# error Must define EDUCATIONAL_NOTES
#endif

// EDUCATIONAL_NOTES(DIAG_ID, EDUCATIONAL_NOTE_FILENAMES...)

EDUCATIONAL_NOTES(non_nominal_no_initializers, "nominal-types.md")
EDUCATIONAL_NOTES(non_nominal_extension, "nominal-types.md")
EDUCATIONAL_NOTES(associated_type_witness_conform_impossible,
"nominal-types.md")

#undef EDUCATIONAL_NOTES
2 changes: 2 additions & 0 deletions include/swift/Basic/DiagnosticOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ class DiagnosticOptions {
/// Descriptive diagnostic output is not intended to be machine-readable.
bool EnableDescriptiveDiagnostics = false;

std::string DiagnosticDocumentationPath = "";

/// Return a hash code of any components from these options that should
/// contribute to a Swift Bridging PCH hash.
llvm::hash_code getPCHHashComponents() const {
Expand Down
4 changes: 4 additions & 0 deletions include/swift/Option/FrontendOptions.td
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@ def show_diagnostics_after_fatal : Flag<["-"], "show-diagnostics-after-fatal">,

def enable_descriptive_diagnostics : Flag<["-"], "enable-descriptive-diagnostics">,
HelpText<"Show descriptive diagnostic information, if available.">;

def diagnostic_documentation_path
: Separate<["-"], "diagnostic-documentation-path">, MetaVarName<"<path>">,
HelpText<"Path to diagnostic documentation resources">;

def enable_swiftcall : Flag<["-"], "enable-swiftcall">,
HelpText<"Enable the use of LLVM swiftcall support">;
Expand Down
25 changes: 25 additions & 0 deletions lib/AST/DiagnosticEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,18 @@ static constexpr const char *const fixItStrings[] = {
"<not a fix-it>",
};

#define EDUCATIONAL_NOTES(DIAG, ...) \
static constexpr const char *const DIAG##_educationalNotes[] = {__VA_ARGS__, \
nullptr};
#include "swift/AST/EducationalNotes.def"

static constexpr const char *const *educationalNotes[LocalDiagID::NumDiags]{
[LocalDiagID::invalid_diagnostic] = {},
#define EDUCATIONAL_NOTES(DIAG, ...) \
[LocalDiagID::DIAG] = DIAG##_educationalNotes,
#include "swift/AST/EducationalNotes.def"
};

DiagnosticState::DiagnosticState() {
// Initialize our per-diagnostic state to default
perDiagnosticBehavior.resize(LocalDiagID::NumDiags, Behavior::Unspecified);
Expand Down Expand Up @@ -951,6 +963,19 @@ void DiagnosticEngine::emitDiagnostic(const Diagnostic &diagnostic) {
}
}
info->ChildDiagnosticInfo = childInfoPtrs;

SmallVector<std::string, 1> educationalNotePaths;
if (useDescriptiveDiagnostics) {
auto associatedNotes = educationalNotes[(uint32_t)diagnostic.getID()];
while (associatedNotes && *associatedNotes) {
SmallString<128> notePath(getDiagnosticDocumentationPath());
llvm::sys::path::append(notePath, *associatedNotes);
educationalNotePaths.push_back(notePath.str());
associatedNotes++;
}
info->EducationalNotePaths = educationalNotePaths;
}

for (auto &consumer : Consumers) {
consumer->handleDiagnostic(SourceMgr, *info);
}
Expand Down
11 changes: 10 additions & 1 deletion lib/Frontend/CompilerInvocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,13 @@ void CompilerInvocation::setMainExecutablePath(StringRef Path) {
llvm::sys::path::remove_filename(LibPath); // Remove /bin
llvm::sys::path::append(LibPath, "lib", "swift");
setRuntimeResourcePath(LibPath.str());

llvm::SmallString<128> DiagnosticDocsPath(Path);
llvm::sys::path::remove_filename(DiagnosticDocsPath); // Remove /swift
llvm::sys::path::remove_filename(DiagnosticDocsPath); // Remove /bin
llvm::sys::path::append(DiagnosticDocsPath, "share", "doc", "swift",
"diagnostics");
DiagnosticOpts.DiagnosticDocumentationPath = DiagnosticDocsPath.str();
}

/// If we haven't explicitly passed -prebuilt-module-cache-path, set it to
Expand Down Expand Up @@ -682,7 +689,9 @@ static bool ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args,
Opts.PrintDiagnosticNames |= Args.hasArg(OPT_debug_diagnostic_names);
Opts.EnableDescriptiveDiagnostics |=
Args.hasArg(OPT_enable_descriptive_diagnostics);

if (Arg *A = Args.getLastArg(OPT_diagnostic_documentation_path)) {
Opts.DiagnosticDocumentationPath = A->getValue();
}
assert(!(Opts.WarningsAsErrors && Opts.SuppressWarnings) &&
"conflicting arguments; should have been caught by driver");

Expand Down
2 changes: 2 additions & 0 deletions lib/Frontend/Frontend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,8 @@ void CompilerInstance::setUpDiagnosticOptions() {
if (Invocation.getDiagnosticOptions().EnableDescriptiveDiagnostics) {
Diagnostics.setUseDescriptiveDiagnostics(true);
}
Diagnostics.setDiagnosticDocumentationPath(
Invocation.getDiagnosticOptions().DiagnosticDocumentationPath);
}

// The ordering of ModuleLoaders is important!
Expand Down
4 changes: 4 additions & 0 deletions lib/Frontend/PrintingDiagnosticConsumer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,10 @@ void PrintingDiagnosticConsumer::handleDiagnostic(SourceManager &SM,
return;

printDiagnostic(SM, Info);
for (auto path : Info.EducationalNotePaths) {
if (auto buffer = SM.getFileSystem()->getBufferForFile(path))
Stream << buffer->get()->getBuffer() << "\n";
}

for (auto ChildInfo : Info.ChildDiagnosticInfo) {
printDiagnostic(SM, *ChildInfo);
Expand Down
15 changes: 15 additions & 0 deletions test/diagnostics/educational-notes.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// RUN: not %target-swift-frontend -enable-descriptive-diagnostics -diagnostic-documentation-path %S/test-docs/ -typecheck %s 2>&1 | %FileCheck %s

// A diagnostic with no educational notes
let x = 1 +
// CHECK: error: expected expression after operator
// CHECK-NOT: {{-+$}}

// A diagnostic with an educational note
extension (Int, Int) {}
// CHECK: error: non-nominal type '(Int, Int)' cannot be extended
// CHECK-NEXT: extension (Int, Int) {}
// CHECK-NEXT: ^ ~~~~~~~~~~
// CHECK-NEXT: Nominal Types
// CHECK-NEXT: -------------
// CHECK-NEXT: Nominal types documentation content
3 changes: 3 additions & 0 deletions test/diagnostics/test-docs/nominal-types.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Nominal Types
-------------
Nominal types documentation content
3 changes: 3 additions & 0 deletions userdocs/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
swift_install_in_component(DIRECTORY diagnostics
DESTINATION "share/doc/swift"
COMPONENT compiler)
7 changes: 7 additions & 0 deletions userdocs/diagnostics/nominal-types.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Nominal types
-------------
In Swift, a type is considered a nominal type if it is named. In other words, it has been defined by declaring the type somewhere in code. Examples of nominal types include classes, structs and enums, all of which must be declared before using them. Nominal types are an important concept in Swift because they can be extended, explicitly initialized using the 'MyType()' syntax, and may conform to protocols.

In contrast, non-nominal types have none of these capabilities. A non-nominal type is any type which is not nominal. They are sometimes called ”structural types” because they are usually obtained by composing other types. Examples include function types like '(Int) -> (String)', tuple types like '(Int, String)', metatypes like 'Int.Type', and special types like 'Any' and 'AnyObject'.

Whether the name of a protocol refers to a nominal or non-nominal type depends on where it appears in code. When used in a declaration or extension like 'extension MyProtocol { … }', 'MyProtocol' refers to a protocol type, which is nominal. This means that it may be extended and conform to protocols. However, when written as the type of a constant or variable, MyProtocol instead refers to a non-nominal, existential type. As a result, code like 'let value: MyProtocol = MyProtocol()' is not allowed because 'MyProtocol' refers to a non-nominal type in this context and cannot be explicitly initialized.