Skip to content

Commit eec3529

Browse files
committed
[Sema] Add fix-it to import RegexBuilder
For code such as the following: ``` let r = Regex { /abc/ } ``` If RegexBuilder has not been imported, emit a specialized diagnostic and fix-it to add `import RegexBuilder` to the file. Unfortunately we're currently prevented from emitting the specialized diagnostic in cases where the builder contains references to RegexBuilder types, such as: ``` let r = Regex { Capture { /abc/ } } ``` This is due to the fact that we bail from CSGen due to the reference to `Capture` being turned into an `ErrorExpr`. We ought to be able to handle solving in the presence of such errors, but for now I'm leaving it as future work. rdar://93176036
1 parent 1577eea commit eec3529

9 files changed

+182
-0
lines changed

include/swift/AST/DiagnosticsSema.def

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4890,6 +4890,9 @@ ERROR(regex_capture_types_failed_to_decode,none,
48904890
"failed to decode capture types for regular expression literal; this may "
48914891
"be a compiler bug", ())
48924892

4893+
ERROR(must_import_regex_builder_module,none,
4894+
"regex builder requires the 'RegexBuilder' module be imported'", ())
4895+
48934896
//------------------------------------------------------------------------------
48944897
// MARK: Type Check Types
48954898
//------------------------------------------------------------------------------

include/swift/AST/KnownIdentifiers.def

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,7 @@ IDENTIFIER(Regex)
258258
IDENTIFIER_(regexString)
259259
IDENTIFIER(version)
260260
IDENTIFIER_(StringProcessing)
261+
IDENTIFIER(RegexBuilder)
261262

262263
// Distributed actors
263264
IDENTIFIER(ActorID)

lib/Sema/CSDiagnostics.cpp

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include "swift/AST/Expr.h"
2727
#include "swift/AST/GenericSignature.h"
2828
#include "swift/AST/Initializer.h"
29+
#include "swift/AST/ImportCache.h"
2930
#include "swift/AST/ParameterList.h"
3031
#include "swift/AST/Pattern.h"
3132
#include "swift/AST/ProtocolConformance.h"
@@ -6460,6 +6461,9 @@ bool ArgumentMismatchFailure::diagnoseAsError() {
64606461
if (diagnosePropertyWrapperMismatch())
64616462
return true;
64626463

6464+
if (diagnoseAttemptedRegexBuilder())
6465+
return true;
6466+
64636467
if (diagnoseTrailingClosureMismatch())
64646468
return true;
64656469

@@ -6704,6 +6708,91 @@ bool ArgumentMismatchFailure::diagnosePropertyWrapperMismatch() const {
67046708
return true;
67056709
}
67066710

6711+
/// Add a fix-it to insert an import of a module.
6712+
static void fixItImport(InFlightDiagnostic &diag, Identifier moduleName,
6713+
DeclContext *dc) {
6714+
auto *SF = dc->getParentSourceFile();
6715+
if (!SF)
6716+
return;
6717+
6718+
SourceLoc insertLoc;
6719+
bool isTrailing = true;
6720+
6721+
// Check if we can insert as the last import statement.
6722+
auto decls = SF->getTopLevelDecls();
6723+
for (auto *decl : decls) {
6724+
auto *importDecl = dyn_cast<ImportDecl>(decl);
6725+
if (!importDecl) {
6726+
if (insertLoc.isValid())
6727+
break;
6728+
continue;
6729+
}
6730+
insertLoc = importDecl->getEndLoc();
6731+
}
6732+
6733+
// If not, insert it as the first decl with a valid source location.
6734+
if (insertLoc.isInvalid()) {
6735+
for (auto *decl : decls) {
6736+
if (auto loc = decl->getStartLoc()) {
6737+
insertLoc = loc;
6738+
isTrailing = false;
6739+
break;
6740+
}
6741+
}
6742+
}
6743+
6744+
// If we didn't resolve to a valid location, give up.
6745+
if (insertLoc.isInvalid())
6746+
return;
6747+
6748+
SmallString<32> insertText;
6749+
if (isTrailing) {
6750+
insertText.append("\n");
6751+
}
6752+
insertText.append("import ");
6753+
insertText.append(moduleName.str());
6754+
if (isTrailing) {
6755+
diag.fixItInsertAfter(insertLoc, insertText);
6756+
} else {
6757+
insertText.append("\n\n");
6758+
diag.fixItInsert(insertLoc, insertText);
6759+
}
6760+
}
6761+
6762+
bool ArgumentMismatchFailure::diagnoseAttemptedRegexBuilder() const {
6763+
auto &ctx = getASTContext();
6764+
6765+
// Should be a lone trailing closure argument.
6766+
if (!Info.isTrailingClosure() || !Info.getArgList()->isUnary())
6767+
return false;
6768+
6769+
// Check if this an application of a Regex initializer, and the user has not
6770+
// imported RegexBuilder.
6771+
auto *ctor = dyn_cast_or_null<ConstructorDecl>(getCallee());
6772+
if (!ctor)
6773+
return false;
6774+
6775+
auto *ctorDC = ctor->getInnermostTypeContext();
6776+
if (!ctorDC || ctorDC->getSelfNominalTypeDecl() != ctx.getRegexDecl())
6777+
return false;
6778+
6779+
// If the RegexBuilder module is loaded, make sure it hasn't been imported.
6780+
// Note this will cause us to diagnose even if another SourceFile has
6781+
// imported RegexBuilder, and its extensions have leaked into this file. This
6782+
// is a longstanding lookup bug, and it's probably a good idea to suggest
6783+
// explicitly importing RegexBuilder regardless in that case.
6784+
if (auto *regexBuilderModule = ctx.getLoadedModule(ctx.Id_RegexBuilder)) {
6785+
auto &importCache = getASTContext().getImportCache();
6786+
if (importCache.isImportedBy(regexBuilderModule, getDC()))
6787+
return false;
6788+
}
6789+
6790+
// Suggest importing RegexBuilder.
6791+
auto diag = emitDiagnostic(diag::must_import_regex_builder_module);
6792+
fixItImport(diag, ctx.Id_RegexBuilder, getDC());
6793+
return true;
6794+
}
6795+
67076796
bool ArgumentMismatchFailure::diagnoseTrailingClosureMismatch() const {
67086797
if (!Info.isTrailingClosure())
67096798
return false;

lib/Sema/CSDiagnostics.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1997,6 +1997,10 @@ class ArgumentMismatchFailure : public ContextualFailure {
19971997
/// so we have to attend to that in diagnostics.
19981998
bool diagnoseMisplacedMissingArgument() const;
19991999

2000+
/// Diagnose an attempt to pass a trailing closure to a Regex initializer
2001+
/// without importing RegexBuilder.
2002+
bool diagnoseAttemptedRegexBuilder() const;
2003+
20002004
protected:
20012005
/// \returns The position of the argument being diagnosed, starting at 1.
20022006
unsigned getArgPosition() const { return Info.getArgPosition(); }
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// RUN: %target-typecheck-verify-swift -enable-bare-slash-regex -disable-availability-checking
2+
3+
import RegexBuilder
4+
5+
extension Regex where Output == Substring {
6+
init(_ x: String) {} // expected-note {{'init(_:)' declared here}}
7+
}
8+
9+
func foo() {
10+
// FIXME: This diagnostic could probably be better, it's not clear we should
11+
// be resolving to init(_ x: String) vs the result builder API and diagnosing
12+
// the fact that Int isn't a RegexComponent.
13+
_ = Regex { // expected-error {{trailing closure passed to parameter of type 'String' that does not accept a closure}}
14+
0
15+
}
16+
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// RUN: %empty-directory(%t)
2+
3+
// Generate some dummy modules to import
4+
// RUN: %target-swift-frontend -emit-module -module-name A -o %t/A.swiftmodule %S/Inputs/dummy.swift
5+
// RUN: %target-swift-frontend -emit-module -module-name B -o %t/B.swiftmodule %S/Inputs/dummy.swift
6+
// RUN: %target-swift-frontend -emit-module -module-name C -o %t/C.swiftmodule %S/Inputs/dummy.swift
7+
8+
// RUN: %target-typecheck-verify-swift -enable-bare-slash-regex -disable-availability-checking -I %t
9+
10+
// REQUIRES: swift_in_compiler
11+
12+
import A
13+
14+
import B
15+
16+
Regex { // expected-error {{regex builder requires the 'RegexBuilder' module be imported'}} {{14:9-9=\nimport RegexBuilder}}
17+
/abc/
18+
}
19+
20+
import C
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// RUN: %target-typecheck-verify-swift -enable-bare-slash-regex -disable-availability-checking
2+
3+
// REQUIRES: swift_in_compiler
4+
5+
struct S {
6+
func foo() {
7+
Regex { // expected-error {{regex builder requires the 'RegexBuilder' module be imported'}} {{5:1-1=import RegexBuilder\n\n}}
8+
/abc/
9+
}
10+
}
11+
}
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// RUN: %target-typecheck-verify-swift -enable-bare-slash-regex -disable-availability-checking
2+
3+
// REQUIRES: swift_in_compiler
4+
5+
Regex {} // expected-error {{regex builder requires the 'RegexBuilder' module be imported'}} {{5:1-1=import RegexBuilder\n\n}}
6+
7+
Regex { // expected-error {{regex builder requires the 'RegexBuilder' module be imported'}} {{5:1-1=import RegexBuilder\n\n}}
8+
/abc/
9+
}
10+
11+
Regex { // expected-error {{regex builder requires the 'RegexBuilder' module be imported'}} {{5:1-1=import RegexBuilder\n\n}}
12+
/abc/
13+
/def/
14+
}
15+
16+
Regex { // expected-error {{regex builder requires the 'RegexBuilder' module be imported'}} {{5:1-1=import RegexBuilder\n\n}}
17+
/abc/
18+
"def"
19+
/g(h)(i)/
20+
}
21+
22+
// FIXME: Unfortunately we bail from CSGen if we end up with an ErrorExpr, so
23+
// don't get a chance to diagnose. We ought to try solving with holes.
24+
// For now at least, this error should at least hopefully nudge users into
25+
// realizing they have a missing import.
26+
Regex {
27+
Capture { // expected-error {{cannot find 'Capture' in scope}}
28+
/abc/
29+
}
30+
}
31+
32+
// It's not clear what the user is doing here so fall back to regular diagnostics.
33+
Regex(a: 0) {
34+
// expected-error@-1 {{cannot convert value of type 'Int' to expected argument type 'String'}}
35+
// expected-error@-2 {{extra trailing closure passed in call}}
36+
/abc/
37+
}

0 commit comments

Comments
 (0)