Diagnose repeat Main//default files as a redundant api file. (#3408)

toolchain/check/check.cpp

@@ -8,6 +8,7 @@
 #include "toolchain/base/pretty_stack_trace_function.h"
 #include "toolchain/base/value_store.h"
 #include "toolchain/check/context.h"
+#include "toolchain/diagnostics/diagnostic_emitter.h"
 #include "toolchain/parse/tree.h"
 #include "toolchain/parse/tree_node_location_translator.h"
 #include "toolchain/sem_ir/file.h"
@@ -17,7 +18,7 @@ namespace Carbon::Check {
 struct UnitInfo {
   explicit UnitInfo(Unit& unit)
       : unit(&unit),
-        translator(unit.tokens, unit.parse_tree),
+        translator(unit.tokens, unit.tokens->filename(), unit.parse_tree),
         err_tracker(*unit.consumer),
         emitter(translator, err_tracker) {}
 
@@ -258,33 +259,46 @@ auto CheckParseTrees(const SemIR::File& builtin_ir,
     // put .impl.carbon on almost all tests (which are in `Main//default`). We
     // should probably get leads direction on filenames before enforcing.
     const auto& packaging = unit_info.unit->parse_tree->packaging_directive();
-    if (packaging) {
-      auto import_key =
-          GetImportKey(unit_info, IdentifierId::Invalid, packaging->names);
-      // Diagnose explicit `Main` uses before they become marked as possible
-      // APIs.
-      if (import_key.first == ExplicitMainName) {
-        CARBON_DIAGNOSTIC(ExplicitMainPackage, Error,
-                          "`Main//default` must omit `package` directive.");
-        CARBON_DIAGNOSTIC(
-            ExplicitMainLibrary, Error,
-            "Use `library` directive in `Main` package libraries.");
-        unit_info.emitter.Emit(packaging->names.node,
-                               import_key.second.empty() ? ExplicitMainPackage
-                                                         : ExplicitMainLibrary);
-        continue;
-      }
+    // An import key formed from the `package` or `library` directive. Or, for
+    // Main//default, a placeholder key.
+    auto import_key = packaging ? GetImportKey(unit_info, IdentifierId::Invalid,
+                                               packaging->names)
+                                // Construct a boring key for Main//default.
+                                : ImportKey{"", ""};
+
+    // Diagnose explicit `Main` uses before they become marked as possible
+    // APIs.
+    if (import_key.first == ExplicitMainName) {
+      CARBON_DIAGNOSTIC(ExplicitMainPackage, Error,
+                        "`Main//default` must omit `package` directive.");
+      CARBON_DIAGNOSTIC(ExplicitMainLibrary, Error,
+                        "Use `library` directive in `Main` package libraries.");
+      unit_info.emitter.Emit(packaging->names.node, import_key.second.empty()
+                                                        ? ExplicitMainPackage
+                                                        : ExplicitMainLibrary);
+      continue;
+    }
 
-      if (packaging->api_or_impl == Parse::Tree::ApiOrImpl::Impl) {
-        continue;
-      }
+    if (packaging && packaging->api_or_impl == Parse::Tree::ApiOrImpl::Impl) {
+      continue;
+    }
 
-      auto [entry, success] = api_map.insert({import_key, &unit_info});
-      if (!success) {
-        // TODO: Cross-reference the source, deal with library, etc.
+    auto [entry, success] = api_map.insert({import_key, &unit_info});
+    if (!success) {
+      llvm::StringRef prev_filename = entry->second->unit->tokens->filename();
+      if (packaging) {
         CARBON_DIAGNOSTIC(DuplicateLibraryApi, Error,
-                          "Library's API declared in more than one file.");
-        unit_info.emitter.Emit(packaging->names.node, DuplicateLibraryApi);
+                          "Library's API previously provided by `{0}`.",
+                          llvm::StringRef);
+        unit_info.emitter.Emit(packaging->names.node, DuplicateLibraryApi,
+                               prev_filename);
+      } else {
+        CARBON_DIAGNOSTIC(DuplicateMainApi, Error,
+                          "Main//default previously provided by `{0}`.",
+                          llvm::StringRef);
+        // Use the invalid node because there's no node to associate with.
+        unit_info.emitter.Emit(Parse::Node::Invalid, DuplicateMainApi,
+                               prev_filename);
       }
     }
   }

toolchain/check/testdata/basics/multifile.carbon

@@ -5,9 +5,13 @@
 // AUTOUPDATE
 
 // --- a.carbon
+package A api;
+
 fn A() {}
 
 // --- b.carbon
+package B api;
+
 fn B() {}
 
 // CHECK:STDOUT: file "a.carbon" {

toolchain/check/testdata/basics/multifile_raw_and_textual_ir.carbon

@@ -9,9 +9,13 @@
 // AUTOUPDATE
 
 // --- a.carbon
+package A api;
+
 fn A() {}
 
 // --- b.carbon
+package B api;
+
 fn B() {}
 
 // CHECK:STDOUT: ---

toolchain/check/testdata/basics/multifile_raw_ir.carbon

@@ -9,9 +9,13 @@
 // AUTOUPDATE
 
 // --- a.carbon
+package A api;
+
 fn A() {}
 
 // --- b.carbon
+package B api;
+
 fn B() {}
 
 // CHECK:STDOUT: ---

toolchain/check/testdata/packages/fail_duplicate_api.carbon

@@ -0,0 +1,60 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// AUTOUPDATE
+// CHECK:STDERR: main2.carbon: ERROR: Main//default previously provided by `main1.carbon`.
+
+// --- main1.carbon
+
+// --- main2.carbon
+
+// --- main_lib1.carbon
+
+library "lib" api;
+
+// --- main_lib2.carbon
+
+// CHECK:STDERR: main_lib2.carbon:[[@LINE+3]]:1: ERROR: Library's API previously provided by `main_lib1.carbon`.
+// CHECK:STDERR: library "lib" api;
+// CHECK:STDERR: ^
+library "lib" api;
+
+// --- package1.carbon
+
+package Package api;
+
+// --- package2.carbon
+
+// CHECK:STDERR: package2.carbon:[[@LINE+3]]:1: ERROR: Library's API previously provided by `package1.carbon`.
+// CHECK:STDERR: package Package api;
+// CHECK:STDERR: ^
+package Package api;
+
+// --- package_lib1.carbon
+
+package Package library "lib" api;
+
+// --- package_lib2.carbon
+
+// CHECK:STDERR: package_lib2.carbon:[[@LINE+3]]:1: ERROR: Library's API previously provided by `package_lib1.carbon`.
+// CHECK:STDERR: package Package library "lib" api;
+// CHECK:STDERR: ^
+package Package library "lib" api;
+
+// CHECK:STDOUT: file "main1.carbon" {
+// CHECK:STDOUT: }
+// CHECK:STDOUT: file "main2.carbon" {
+// CHECK:STDOUT: }
+// CHECK:STDOUT: file "main_lib1.carbon" {
+// CHECK:STDOUT: }
+// CHECK:STDOUT: file "main_lib2.carbon" {
+// CHECK:STDOUT: }
+// CHECK:STDOUT: file "package1.carbon" {
+// CHECK:STDOUT: }
+// CHECK:STDOUT: file "package2.carbon" {
+// CHECK:STDOUT: }
+// CHECK:STDOUT: file "package_lib1.carbon" {
+// CHECK:STDOUT: }
+// CHECK:STDOUT: file "package_lib2.carbon" {
+// CHECK:STDOUT: }

toolchain/check/testdata/packages/fail_import_invalid.carbon

@@ -11,9 +11,7 @@
 // CHECK:STDERR: ^
 import Main;
 
-// --- main_lib.carbon
-
-// CHECK:STDERR: main_lib.carbon:[[@LINE+3]]:1: ERROR: Imports from the current package must omit the package name.
+// CHECK:STDERR: main.carbon:[[@LINE+3]]:1: ERROR: Imports from the current package must omit the package name.
 // CHECK:STDERR: import Main library "lib";
 // CHECK:STDERR: ^
 import Main library "lib";
@@ -77,16 +75,15 @@ package Implicit library "lib" impl;
 import Implicit library "lib";
 
 // --- not_found.carbon
+package NotFound api;
 
 // CHECK:STDERR: not_found.carbon:[[@LINE+3]]:1: ERROR: Imported API not found.
-// CHECK:STDERR: import NotFound;
+// CHECK:STDERR: import ImportNotFound;
 // CHECK:STDERR: ^
-import NotFound;
+import ImportNotFound;
 
 // CHECK:STDOUT: file "main.carbon" {
 // CHECK:STDOUT: }
-// CHECK:STDOUT: file "main_lib.carbon" {
-// CHECK:STDOUT: }
 // CHECK:STDOUT: file "not_main.carbon" {
 // CHECK:STDOUT: }
 // CHECK:STDOUT: file "this.carbon" {

toolchain/diagnostics/diagnostic_kind.def

@@ -118,6 +118,7 @@ CARBON_DIAGNOSTIC_KIND(SemanticsTodo)
 
 // Package/import checking diagnostics.
 CARBON_DIAGNOSTIC_KIND(DuplicateLibraryApi)
+CARBON_DIAGNOSTIC_KIND(DuplicateMainApi)
 CARBON_DIAGNOSTIC_KIND(LibraryApiNotFound)
 CARBON_DIAGNOSTIC_KIND(ImportNotFound)
 CARBON_DIAGNOSTIC_KIND(ImportCycleDetected)

toolchain/parse/tree_node_location_translator.h

@@ -12,16 +12,25 @@ namespace Carbon::Parse {
 class NodeLocationTranslator : public DiagnosticLocationTranslator<Node> {
  public:
   explicit NodeLocationTranslator(const Lex::TokenizedBuffer* tokens,
+                                  llvm::StringRef filename,
                                   const Tree* parse_tree)
-      : token_translator_(tokens), parse_tree_(parse_tree) {}
+      : token_translator_(tokens),
+        filename_(filename),
+        parse_tree_(parse_tree) {}
 
   // Map the given token into a diagnostic location.
   auto GetLocation(Node node) -> DiagnosticLocation override {
+    // Support the invalid token as a way to emit only the filename, when there
+    // is no line association.
+    if (!node.is_valid()) {
+      return {.file_name = filename_};
+    }
     return token_translator_.GetLocation(parse_tree_->node_token(node));
   }
 
  private:
   Lex::TokenLocationTranslator token_translator_;
+  llvm::StringRef filename_;
   const Tree* parse_tree_;
 };