Diagnose incorrect uses of module selectors

# Conflicts:
#	lib/Parse/ParseDecl.cpp

Author: beccadax

include/swift/AST/DiagnosticsParse.def

@@ -95,6 +95,9 @@ ERROR(expected_identifier_in_module_selector,none,
       "expected identifier in module selector", ())
 ERROR(module_selector_not_allowed_here,none,
       "name cannot be qualified with module selector here", ())
+ERROR(module_selector_not_allowed_in_decl,none,
+      "%select{%1|name of %1 declaration}0 cannot be qualified with module "
+      "selector", (bool, StringRef))
 
 //------------------------------------------------------------------------------
 // MARK: Lexer diagnostics

include/swift/Parse/Parser.h

@@ -645,6 +645,21 @@ class Parser {
              Context.getIdentifier(tok.getText()));
   }
 
+  /// If the next two tokens appear to form a module selector, consume them and
+  /// diagnose an error. Call this method before consuming an identifier that
+  /// should *not* have a module selector before it.
+  ///
+  /// \param KindName A string describing the name or declaration being written.
+  /// \param IsDef If \c true, the identifier defines a declaration's name, and
+  /// \p KindName should be a string describing the declaration; if \c false,
+  /// the identifier does something else, and \p KindName should be a more
+  /// complete string.
+  ///
+  /// \returns true if a module selector was consumed and an error was diagnosed;
+  /// false otherwise.
+  bool
+  diagnoseAndConsumeIfModuleSelector(StringRef KindName, bool IsDef = true);
+
   /// Retrieve the location just past the end of the previous
   /// source location.
   SourceLoc getEndOfPreviousLoc() const;

lib/Parse/ParseDecl.cpp

@@ -1968,6 +1968,10 @@ bool Parser::parseNewDeclAttribute(DeclAttributes &Attributes, SourceLoc AtLoc,
       return false;
     }
 
+    diagnoseAndConsumeIfModuleSelector(
+        "Objective-C class name in @_swift_native_objc_runtime_base",
+        /*IsDef=*/true);
+
     if (Tok.isNot(tok::identifier)) {
       diagnose(Loc, diag::swift_native_objc_runtime_base_must_be_identifier);
       return false;
@@ -2588,6 +2592,8 @@ bool Parser::parseNewDeclAttribute(DeclAttributes &Attributes, SourceLoc AtLoc,
   }
 
   case DAK_ProjectedValueProperty: {
+    diagnoseAndConsumeIfModuleSelector("@_projectedValueProperty attribute");
+
     if (!consumeIf(tok::l_paren)) {
       diagnose(Loc, diag::attr_expected_lparen, AttrName,
                DeclAttribute::isDeclModifier(DK));
@@ -4807,10 +4813,28 @@ ParserStatus Parser::parseInheritance(
   return Status;
 }
 
+bool Parser::diagnoseAndConsumeIfModuleSelector(StringRef KindName,
+                                               bool IsDefinition) {
+  if (!Context.LangOpts.EnableExperimentalModuleSelector ||
+      peekToken().isNot(tok::colon_colon))
+    return false;
+
+  // Diagnose an error and consume the module selector so we can continue.
+  SourceLoc start = consumeToken();
+  SourceLoc end = consumeToken(tok::colon_colon);
+  diagnose(end, diag::module_selector_not_allowed_in_decl, IsDefinition,
+           KindName)
+      .fixItRemove({start, end});
+
+  return true;
+}
+
 static ParserStatus
 parseIdentifierDeclName(Parser &P, Identifier &Result, SourceLoc &Loc,
                         StringRef DeclKindName,
                         llvm::function_ref<bool(const Token &)> canRecover) {
+  P.diagnoseAndConsumeIfModuleSelector(DeclKindName);
+
   if (P.Tok.is(tok::identifier)) {
     Loc = P.consumeIdentifier(Result, /*diagnoseDollarPrefix=*/true);
 
@@ -5725,6 +5749,9 @@ static ParameterList *parseOptionalAccessorArgument(SourceLoc SpecifierLoc,
   if (SpecifierLoc.isValid() && P.Tok.is(tok::l_paren)) {
     SyntaxParsingContext ParamCtx(P.SyntaxContext, SyntaxKind::AccessorParameter);
     StartLoc = P.consumeToken(tok::l_paren);
+
+    P.diagnoseAndConsumeIfModuleSelector("accessor parameter");
+
     if (P.Tok.isNot(tok::identifier)) {
       P.diagnose(P.Tok, diag::expected_accessor_parameter_name,
                  Kind == AccessorKind::Set ? 0 :
@@ -7209,6 +7236,8 @@ Parser::parseDeclEnumCase(ParseDeclOptions Flags,
       break;
     }
 
+    diagnoseAndConsumeIfModuleSelector("enum 'case'");
+
     if (Tok.is(tok::identifier)) {
       Status |= parseIdentifierDeclName(
           *this, Name, NameLoc, "enum 'case'", [](const Token &next) {
@@ -8124,6 +8153,10 @@ parsePrecedenceGroupNameList(Parser &P, Fn takeGroupName) {
       return makeParserCodeCompletionStatus();
     }
 
+    // TODO: We could support module selectors for precedence groups if we
+    //       implemented the lookup for it.
+    P.diagnoseAndConsumeIfModuleSelector("precedence group specifier",
+                                         /*isDef=*/false);
     DeclNameLoc nameLoc;
     auto name = P.parseDeclNameRef(nameLoc,
         diag::expected_group_name_in_precedencegroup_list, {});
@@ -8265,6 +8298,8 @@ Parser::parseDeclPrecedenceGroup(ParseDeclOptions flags,
     return nullptr;
   }
 
+  diagnoseAndConsumeIfModuleSelector("precedence group");
+
   Identifier name;
   SourceLoc nameLoc;
   if (parseIdentifier(name, nameLoc, /*diagnoseDollarPrefix=*/true,

lib/Parse/ParseExpr.cpp

@@ -1642,6 +1642,10 @@ ParserResult<Expr> Parser::parseExprPrimary(Diag<> ID, bool isExprBasic) {
         // call pattern.
         peekToken().isNot(tok::period, tok::period_prefix, tok::l_paren)) {
       DeferringContextRAII Deferring(*SyntaxContext);
+
+      diagnoseAndConsumeIfModuleSelector(InVarOrLetPattern != IVOLP_InVar
+                                         ? "constant" : "variable");
+
       Identifier name;
       SourceLoc loc = consumeIdentifier(name, /*diagnoseDollarPrefix=*/false);
       auto introducer = (InVarOrLetPattern != IVOLP_InVar
@@ -2576,9 +2580,23 @@ ParserStatus Parser::parseClosureSignatureIfPresent(
 
       // Okay, we have a closure signature.
     } else if (Tok.isIdentifierOrUnderscore() || Tok.is(tok::code_complete)) {
-      // Parse identifier (',' identifier)*
+      // Parse module-selector? identifier (',' module-selector? identifier)*
+      // The module selectors aren't legal, but we want to diagnose them if
+      // they're present.
+      if (Context.LangOpts.EnableExperimentalModuleSelector &&
+          peekToken().is(tok::colon_colon)) {
+        consumeToken();
+        consumeToken(tok::colon_colon);
+      }
+
       consumeToken();
       while (consumeIf(tok::comma)) {
+        if (Context.LangOpts.EnableExperimentalModuleSelector &&
+            peekToken().is(tok::colon_colon)) {
+          consumeToken();
+          consumeToken(tok::colon_colon);
+        }
+
         if (Tok.isIdentifierOrUnderscore() || Tok.is(tok::code_complete)) {
           consumeToken();
           continue;
@@ -2655,8 +2673,11 @@ ParserStatus Parser::parseClosureSignatureIfPresent(
             diagnose(Tok, diag::attr_unowned_expected_rparen);
         }
       } else if (Tok.isAny(tok::identifier, tok::kw_self, tok::code_complete) &&
-                 peekToken().isAny(tok::equal, tok::comma, tok::r_square)) {
+                 peekToken().isAny(tok::equal, tok::comma, tok::r_square,
+                                   tok::colon_colon)) {
         // "x = 42", "x," and "x]" are all strong captures of x.
+        // "x::" is not permitted, but we want to diagnose it as though it were
+        // a strong capture.
       } else {
         diagnose(Tok, diag::expected_capture_specifier);
         skipUntil(tok::comma, tok::r_square);
@@ -2674,10 +2695,17 @@ ParserStatus Parser::parseClosureSignatureIfPresent(
 
       // The thing being capture specified is an identifier, or as an identifier
       // followed by an expression.
+
       Expr *initializer;
       Identifier name;
       SourceLoc nameLoc = Tok.getLoc();
       SourceLoc equalLoc;
+
+      // FIXME: It'd be nice to be able to capture with a module selector, but
+      // define a local name; however, that would complicate the equals check
+      // here.
+      diagnoseAndConsumeIfModuleSelector("captured variable");
+
       if (peekToken().isNot(tok::equal)) {
         // If this is the simple case, then the identifier is both the name and
         // the expression to capture.
@@ -2782,6 +2810,9 @@ ParserStatus Parser::parseClosureSignatureIfPresent(
       bool HasNext;
       do {
         SyntaxParsingContext ClParamCtx(SyntaxContext, SyntaxKind::ClosureParam);
+
+        diagnoseAndConsumeIfModuleSelector("parameter");
+
         if (Tok.isNot(tok::identifier, tok::kw__, tok::code_complete)) {
           diagnose(Tok, diag::expected_closure_parameter_name);
           status.setIsParseError();

lib/Parse/ParseGeneric.cpp

@@ -68,6 +68,8 @@ Parser::parseGenericParametersBeforeWhere(SourceLoc LAngleLoc,
       attributes.add(new (Context) RawDocCommentAttr(Tok.getCommentRange()));
     parseDeclAttributeList(attributes);
 
+    diagnoseAndConsumeIfModuleSelector("generic parameter");
+
     // Parse the name of the parameter.
     Identifier Name;
     SourceLoc NameLoc;

lib/Parse/ParsePattern.cpp

@@ -204,6 +204,12 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
          rightParenLoc.isInvalid() && "Must start with empty state");
   SyntaxParsingContext ParamClauseCtx(SyntaxContext, SyntaxKind::ParameterClause);
 
+  // Operators and closures cannot have API names. Enum elements cannot have
+  // internal names.
+  bool canHaveLabels = !(paramContext == ParameterContextKind::Operator ||
+                         paramContext == ParameterContextKind::Closure);
+  bool canHaveInternalName = paramContext != ParameterContextKind::EnumElement;
+
   // Consume the starting '(';
   leftParenLoc = consumeToken(tok::l_paren);
 
@@ -325,22 +331,25 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
         .fixItReplace(Tok.getLoc(), "`" + Tok.getText().str() + "`");
     }
 
+    diagnoseAndConsumeIfModuleSelector(
+        canHaveLabels ? "argument label" : "parameter",
+        /*isDef=*/!canHaveLabels);
+
     if (startsParameterName(isClosure)) {
       // identifier-or-none for the first name
       param.FirstNameLoc = consumeArgumentLabel(param.FirstName,
                                                 /*diagnoseDollarPrefix=*/!isClosure);
 
+      diagnoseAndConsumeIfModuleSelector("parameter");
+
       // identifier-or-none? for the second name
       if (Tok.canBeArgumentLabel())
         param.SecondNameLoc = consumeArgumentLabel(param.SecondName,
                                                    /*diagnoseDollarPrefix=*/true);
 
-      // Operators, closures, and enum elements cannot have API names.
-      if ((paramContext == ParameterContextKind::Operator ||
-           paramContext == ParameterContextKind::Closure ||
-           paramContext == ParameterContextKind::EnumElement) &&
-          !param.FirstName.empty() &&
-          param.SecondNameLoc.isValid()) {
+      // Do we have two names in a parameter that can't have both?
+      if (!(canHaveLabels && canHaveInternalName) &&
+          !param.FirstName.empty() && param.SecondNameLoc.isValid()) {
         enum KeywordArgumentDiagnosticContextKind {
           Operator    = 0,
           Closure     = 1,
@@ -1101,6 +1110,12 @@ ParserResult<Pattern> Parser::parsePattern() {
   auto introducer = (InVarOrLetPattern != IVOLP_InVar
                      ? VarDecl::Introducer::Let
                      : VarDecl::Introducer::Var);
+  StringRef declNameKind = introducer == VarDecl::Introducer::Let
+                         ? "constant" : "variable";
+
+  // If there's a module selector here, consume and remove it.
+  diagnoseAndConsumeIfModuleSelector(declNameKind);
+
   switch (Tok.getKind()) {
   case tok::l_paren:
     return parsePatternTuple();
@@ -1120,19 +1135,17 @@ ParserResult<Pattern> Parser::parsePattern() {
     }
     PatternCtx.setCreateSyntax(SyntaxKind::WildcardPattern);
     return makeParserResult(new (Context) AnyPattern(consumeToken(tok::kw__)));
-    
+
   case tok::identifier: {
     PatternCtx.setCreateSyntax(SyntaxKind::IdentifierPattern);
     Identifier name;
     SourceLoc loc = consumeIdentifier(name, /*diagnoseDollarPrefix=*/true);
     if (Tok.isIdentifierOrUnderscore() && !Tok.isContextualDeclKeyword())
-      diagnoseConsecutiveIDs(name.str(), loc,
-                             introducer == VarDecl::Introducer::Let
-                             ? "constant" : "variable");
+      diagnoseConsecutiveIDs(name.str(), loc, declNameKind);
 
     return makeParserResult(createBindingFromPattern(loc, name, introducer));
   }
-    
+
   case tok::code_complete:
     if (!CurDeclContext->isTypeContext()) {
       // This cannot be an overridden property, so just eat the token. We cannot
@@ -1168,7 +1181,7 @@ ParserResult<Pattern> Parser::parsePattern() {
     return makeParserResult(
         new (Context) BindingPattern(varLoc, isLet, subPattern.get()));
   }
-      
+
   default:
     if (Tok.isKeyword() &&
         (peekToken().is(tok::colon) || peekToken().is(tok::equal))) {
@@ -1201,6 +1214,10 @@ Parser::parsePatternTupleElement() {
   Identifier Label;
   SourceLoc LabelLoc;
 
+  StringRef declNameKind = InVarOrLetPattern != IVOLP_InVar
+                         ? "constant" : "variable";
+  diagnoseAndConsumeIfModuleSelector(declNameKind);
+
   // If the tuple element has a label, parse it.
   if (Tok.is(tok::identifier) && peekToken().is(tok::colon)) {
     LabelLoc = consumeIdentifier(Label, /*diagnoseDollarPrefix=*/true);

test/NameLookup/module_selector.swift

@@ -125,3 +125,81 @@ func whitespace() {
   ::
   print // expected-error{{expression resolves to an unused function}}
 }
+
+// Error cases
+
+func main::decl1( // expected-error {{name of function declaration cannot be qualified with module selector}}
+  main::p1: main::A, // expected-error {{argument label cannot be qualified with module selector}}
+  main::label p2: main::A, // expected-error {{argument label cannot be qualified with module selector}}
+  label main::p3: main::A // expected-error {{name of parameter declaration cannot be qualified with module selector}}
+) {
+  let main::decl1a = "a" // expected-error {{name of constant declaration cannot be qualified with module selector}} expected-warning {{never used}}
+  var main::decl1b = "b" // expected-error {{name of variable declaration cannot be qualified with module selector}} expected-warning {{never used}}
+  let (main::decl1c, main::decl1d) = ("c", "d") // expected-error {{name of constant declaration cannot be qualified with module selector}} expected-error {{name of constant declaration cannot be qualified with module selector}} expected-warning 2{{never used}}
+
+  if let (main::decl1e, main::decl1f) = Optional(("e", "f")) {} // expected-error {{name of constant declaration cannot be qualified with module selector}} expected-error {{name of constant declaration cannot be qualified with module selector}} expected-warning 2{{never used}}
+  guard let (main::decl1g, main::decl1h) = Optional(("g", "h")) else { return }  // expected-error {{name of constant declaration cannot be qualified with module selector}} expected-error {{name of constant declaration cannot be qualified with module selector}} expected-warning {{never used}}
+
+  switch Optional(main::decl1g) { // FIXME expecting an error later
+  case Optional.some(let main::decl1i): // expected-error {{name of constant declaration cannot be qualified with module selector}} expected-warning {{never used}}
+    break
+  case .none:
+    break
+  }
+
+  switch Optional(main::decl1g) { // FIXME expecting an error later
+  case let Optional.some(main::decl1j): // expected-error {{name of constant declaration cannot be qualified with module selector}} expected-warning {{never used}}
+    break
+  case .none:
+    break
+  }
+
+  switch Optional(main::decl1g) {
+  case let main::decl1k?: // expected-error {{name of constant declaration cannot be qualified with module selector}} expected-warning {{never used}}
+    break
+  case .none:
+    break
+  }
+
+  for main::decl1l in "lll" {} // expected-error {{name of constant declaration cannot be qualified with module selector}} expected-warning {{never used}}
+
+  "lll".forEach { [main::magnitude] // expected-error {{name of captured variable declaration cannot be qualified with module selector}} expected-warning {{never used}}
+    main::elem in print(elem) // expected-error {{name of parameter declaration cannot be qualified with module selector}}
+  }
+  "lll".forEach { (main::elem) in print(elem) } // expected-error {{name of parameter declaration cannot be qualified with module selector}}
+  "lll".forEach { (main::elem) -> Void in print(elem) } // expected-error {{name of parameter declaration cannot be qualified with module selector}}
+  "lll".forEach { (main::elem: Character) -> Void in print(elem) } // expected-error {{name of parameter declaration cannot be qualified with module selector}}
+}
+enum main::decl2 { // expected-error {{name of enum declaration cannot be qualified with module selector}}
+  case main::decl2a // expected-error {{name of enum 'case' declaration cannot be qualified with module selector}}
+}
+struct main::decl3 {} // expected-error {{name of struct declaration cannot be qualified with module selector}}
+class main::decl4<main::T> {} // expected-error {{name of class declaration cannot be qualified with module selector}} expected-error {{name of generic parameter declaration cannot be qualified with module selector}}
+typealias main::decl5 = main::Bool // expected-error {{name of typealias declaration cannot be qualified with module selector}}
+protocol main::decl6 { // expected-error {{name of protocol declaration cannot be qualified with module selector}}
+  associatedtype main::decl6a // expected-error {{name of associatedtype declaration cannot be qualified with module selector}}
+}
+let main::decl7 = 7 // expected-error {{name of constant declaration cannot be qualified with module selector}}
+var main::decl8 = 8 { // expected-error {{name of variable declaration cannot be qualified with module selector}}
+  willSet(main::newValue) {} // expected-error {{name of accessor parameter declaration cannot be qualified with module selector}}
+  didSet(main::oldValue) {} // expected-error {{name of accessor parameter declaration cannot be qualified with module selector}}
+}
+
+struct Parent {
+  func main::decl1() {} // expected-error {{name of function declaration cannot be qualified with module selector}}
+  enum main::decl2 { // expected-error {{name of enum declaration cannot be qualified with module selector}}
+    case main::decl2a // expected-error {{name of enum 'case' declaration cannot be qualified with module selector}}
+  }
+  struct main::decl3 {} // expected-error {{name of struct declaration cannot be qualified with module selector}}
+  class main::decl4 {} // expected-error {{name of class declaration cannot be qualified with module selector}}
+  typealias main::decl5 = main::Bool // expected-error {{name of typealias declaration cannot be qualified with module selector}}
+}
+
+@_swift_native_objc_runtime_base(main::BaseClass) // expected-error {{Objective-C class name in @_swift_native_objc_runtime_base}}
+class C1 {}
+
+infix operator <<<<< : Swift::AdditionPrecedence // expected-error {{precedence group specifier cannot be qualified with module selector}}
+
+precedencegroup main::PG1 { // expected-error {{name of precedence group declaration cannot be qualified with module selector}}
+  higherThan: Swift::AdditionPrecedence // expected-error {{precedence group specifier cannot be qualified with module selector}}
+}