All files / PARSE/4-Resolve ConflictDetector.ts

95.09% Statements 97/102
89.23% Branches 58/65
100% Functions 20/20
94.84% Lines 92/97

Press n or j to go to the next uncovered block, b, p or k for the previous block.

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217 218 219 220 221 222 223 224 225 226 227 228 229 230 231 232 233 234 235 236 237 238 239 240 241 242 243 244 245 246 247 248 249 250 251 252 253 254 255 256 257 258 259 260 261 262 263 264 265 266 267 268 269 270 271 272 273 274 275 276 277 278 279 280 281 282 283 284 285 286 287 288 289 290 291 292 293 294 295 296 297 298 299 300 301 302 303 304 305 306 307 308 309 310 311 312 313 314 315 316 317 318 319 320 321 322 323 324 325 326 327 328 329 330 331 332 333 334 335 336 337 338 339 340 341 342 343 344 345 346 347 348 349 350 351 352 353 354 355 356 357 358 359 360 361 362 363 364 365 366 367 368 369 370 371 372 373 374 375 376 377 378 379 380 381 382 383 384 385 386 387 388 389 390 391 392 393 394 395 396 397 398 399 400 401 402 403 404 405 406 407 408 409 410 411 412 413 414 415 416 417 418 419 420 421 422 423 424 425 426 427 428 429 430 431 432 433 434 435 436 437 438 439 440 441 442 443 444 445 446 447 448 449 450 451 452 453 454 455 456 457 458 459 460                                                                                      3241x 3241x 3241x   3241x 3241x 6817x 3241x 3241x   3241x 7156x           7156x   113x 113x 9x       3241x                   7156x                               9723x 9723x 7275x 7275x 111x   7164x     9723x                       218x       60x   158x         158x                                 106x   218x       106x 105x     1x 2x 2x 2x       1x                               5x             5x         5x                                         10x         10x                                   20x                     113x 232x     113x   7x     106x       106x       106x 1x       105x 216x   105x 216x   105x 216x           105x 130x 130x     105x       4x                       4x       10x         4x                                         101x       101x 5x       96x 1x     95x                                   101x 41x       60x   60x 102x 84x                           18x 13x     5x       5x                 5x       5x                               55x                               60x   60x 123x           123x 123x 123x 21x   102x       60x          
/**
 * Symbol conflicts — one of the twelve Tier 2 cross-file facts (#1511).
 *
 * A conflict is cross-file by construction: it exists only when two files
 * define the same name, so no per-file pass can see one. It lived on
 * `SymbolTable` because that is where symbols accumulated, which made the fact
 * a property of an accumulator rather than of the program — the answer depended
 * on how much had been inserted by the time it was asked.
 *
 * This detector holds no state. It is handed the three languages' symbols and
 * returns the conflicts among them, so the same input always yields the same
 * answer. `Program` supplies that input, and after 1.4 nothing else derives it.
 *
 * The three arrays stay separate rather than arriving pre-merged. Report order
 * is name-first-seen across C-Next, then C, then C++, and each name's
 * definitions are listed in that same language order. Both orderings are
 * observable — they decide which definition a diagnostic reports at — so
 * merging at the call site would hand an unstated invariant to every caller.
 */
 
import DeclarationSite from "../../utils/DeclarationSite";
import ScopeUtils from "../../utils/ScopeUtils";
import SymbolRegistry from "../3-Declare/SymbolRegistry";
import ESourceLanguage from "../../utils/types/ESourceLanguage";
import type IConflict from "../../types/IConflict";
import type TSymbol from "../../types/symbols/TSymbol";
import type TCSymbol from "../../types/symbols/c/TCSymbol";
import type TCppSymbol from "../../types/symbols/cpp/TCppSymbol";
import type TAnySymbol from "../../types/symbols/TAnySymbol";
 
class ConflictDetector {
  /**
   * Every conflict among these symbols.
   *
   * Names are visited in the order first seen — C-Next, then C, then C++ — and
   * each name's definitions gathered in that same order.
   */
  static detect(
    registry: SymbolRegistry | null,
    cnext: ReadonlyArray<TSymbol>,
    c: ReadonlyArray<TCSymbol>,
    cpp: ReadonlyArray<TCppSymbol>,
  ): IConflict[] {
    const cnextByName = ConflictDetector.indexByName(cnext);
    const cByName = ConflictDetector.indexByName(c);
    const cppByName = ConflictDetector.indexByName(cpp);
 
    const conflicts: IConflict[] = [];
    const allNames = new Set<string>();
    for (const name of cnextByName.keys()) allNames.add(name);
    for (const name of cByName.keys()) allNames.add(name);
    for (const name of cppByName.keys()) allNames.add(name);
 
    for (const name of allNames) {
      const symbols = ConflictDetector.overloadsOf(
        name,
        cnextByName,
        cByName,
        cppByName,
      );
      if (symbols.length <= 1) continue;
 
      const conflict = ConflictDetector.detectConflict(registry, symbols);
      if (conflict) {
        conflicts.push(conflict);
      }
    }
 
    return conflicts;
  }
 
  /** Every definition of one name, in C-Next, then C, then C++ order. */
  private static overloadsOf(
    name: string,
    cnextByName: ReadonlyMap<string, TAnySymbol[]>,
    cByName: ReadonlyMap<string, TAnySymbol[]>,
    cppByName: ReadonlyMap<string, TAnySymbol[]>,
  ): TAnySymbol[] {
    return [
      ...(cnextByName.get(name) ?? []),
      ...(cByName.get(name) ?? []),
      ...(cppByName.get(name) ?? []),
    ];
  }
 
  /**
   * Bare name to its definitions, insertion-ordered.
   *
   * Bare, not transpiled: a scoped member is found by the name it carries
   * inside its scope, which is the name a colliding C symbol would share.
   */
  private static indexByName(
    symbols: ReadonlyArray<TAnySymbol>,
  ): Map<string, TAnySymbol[]> {
    const index = new Map<string, TAnySymbol[]>();
    for (const symbol of symbols) {
      const existing = index.get(symbol.name);
      if (existing) {
        existing.push(symbol);
      } else {
        index.set(symbol.name, [symbol]);
      }
    }
    return index;
  }
 
  /**
   * Issue #221: function parameters must not count as conflicting definitions.
   * They have a parent, but their name is not qualified with the parent prefix.
   *
   * Only C/C++ symbols are filtered. A C-Next variable is always kept: at this
   * point a scope-level variable and a function parameter are indistinguishable,
   * so the original code returned true down both of its branches.
   */
  private static isNotFunctionParameter(def: TAnySymbol): boolean {
    if (
      def.sourceLanguage === ESourceLanguage.CNext &&
      def.kind === "variable"
    ) {
      return true;
    }
    Iif ("parent" in def && def.parent) {
      // A non-variable with a parent is a real definition; a variable with a
      // parent may be a function parameter, so it is dropped.
      return def.kind !== "variable";
    }
    return true;
  }
 
  /**
   * True when every definition is a C++ function and all their signatures
   * differ -- overloads, which are legal rather than a conflict.
   *
   * Currently redundant (#1180): no path in detectConflict reports a conflict
   * between two C++ symbols, so an all-C++ group returns null whether this
   * short-circuits or falls through. Verified by mutation -- forcing this to
   * false left all 49 of the then-owner's tests passing. Carried here
   * rather than deleted, because which way to resolve it (drop the branch, or
   * add the same-signature conflict it implies) is a behavior decision.
   */
  private static areAllDistinctCppOverloads(
    globalDefinitions: TAnySymbol[],
  ): boolean {
    const cppFunctions = globalDefinitions.filter(
      (s) =>
        s.sourceLanguage === ESourceLanguage.Cpp &&
        s.kind === "function" &&
        "parameters" in s,
    );
    if (cppFunctions.length !== globalDefinitions.length) {
      return false;
    }
 
    const signatures = cppFunctions.map((f) => {
      Eif ("parameters" in f && f.parameters) {
        const params = f.parameters as ReadonlyArray<{ type?: string }>;
        return params.map((p) => p.type ?? "").join(",");
      }
      return "";
    });
    return new Set(signatures).size === cppFunctions.length;
  }
 
  /**
   * The blocks declaring the scope these symbols belong to, or "" at global scope.
   *
   * #1334: ADR-016 lets a scope be reopened, so the members that collide may sit
   * in different blocks of a scope spread across several files. Naming only the
   * member definitions leaves the reader to find those blocks themselves.
   */
  private static scopeDeclarationNote(
    registry: SymbolRegistry | null,
    symbol: TSymbol,
  ): string {
    // #1298: the symbol names its scope by path; the object -- and the mutable
    // `declarationSites` on it -- is one registry lookup away.
    const scope = registry?.getScope(symbol.scopePath) ?? null;
    // The global-scope disjunct is stated, not merely implied. It never decides
    // the result -- the only `declarationSites` writer targets a grammar-
    // guaranteed non-empty identifier, so the global scope's set is always empty
    // and the third disjunct would catch it -- but "a global symbol gets no
    // declaration note" is the intent, and `getScope("")` returns the global
    // scope object rather than null, so nothing else says it (#1298 review).
    Eif (
      scope === null ||
      ScopeUtils.isGlobalScopePath(symbol.scopePath) ||
      scope.declarationSites.size === 0
    ) {
      return "";
    }
    // Sorted through DeclarationSite, not `.sort()`: these keys end in a line
    // number, and a text sort orders `:10` ahead of `:3` (SonarCloud S2871).
    const sites = [...scope.declarationSites]
      .sort(DeclarationSite.compare)
      .map(DeclarationSite.displaySite);
    // Indented: the CLI's error format treats an unindented line as a new
    // diagnostic, so an unindented header here is dropped by any consumer that
    // parses stderr -- including the test harness, which kept the sites and lost
    // the sentence introducing them.
    return `\n  scope '${scope.name}' is declared in:\n    ${sites.join("\n    ")}`;
  }
 
  /**
   * The language a definition came from, as a reader knows it.
   *
   * Private because this detector is the only consumer today; promote it beside
   * ESourceLanguage if a second one appears.
   */
  private static languageName(symbol: TAnySymbol): string {
    const names: Record<ESourceLanguage, string> = {
      [ESourceLanguage.CNext]: "C-Next",
      [ESourceLanguage.C]: "C",
      [ESourceLanguage.Cpp]: "C++",
    };
    return names[symbol.sourceLanguage];
  }
 
  /**
   * Where a definition is, as a symbol carries it.
   *
   * #1334: the two conflict producers formatted this differently, so the same fact
   * printed two ways depending on which path found it. The rendering itself lives
   * in DeclarationSite -- see there for why it is a basename and why the ordering
   * needs a comparator.
   *
   * Line-granular ON PURPOSE, not for want of a column. #1318 gave every symbol a
   * `span`, and the three diagnostics below report `span.column`; this renders the
   * `file:line` form that `declarationSites` is keyed on, so adding a column here
   * would stop the two matching. (This comment previously claimed symbols carried
   * no column, and was stacked above `languageName` rather than this function.)
   */
  private static locationOf(symbol: TAnySymbol): string {
    return DeclarationSite.display(symbol.sourceFile, symbol.span.line);
  }
 
  /**
   * Detect if a set of symbols with the same name represents a conflict
   */
  private static detectConflict(
    registry: SymbolRegistry | null,
    symbols: TAnySymbol[],
  ): IConflict | null {
    // Filter out pure declarations (extern in C) - they don't count as definitions
    const definitions = symbols.filter(
      (s) => !("isDeclaration" in s && s.isDeclaration),
    );
 
    if (definitions.length <= 1) {
      // 0 or 1 definitions = no conflict
      return null;
    }
 
    const globalDefinitions = definitions.filter(
      ConflictDetector.isNotFunctionParameter,
    );
 
    Iif (globalDefinitions.length <= 1) {
      return null;
    }
 
    if (ConflictDetector.areAllDistinctCppOverloads(globalDefinitions)) {
      return null;
    }
 
    // Check for cross-language conflict (C-Next vs C or C++)
    const cnextDefs = globalDefinitions.filter(
      (s) => s.sourceLanguage === ESourceLanguage.CNext,
    );
    const cDefs = globalDefinitions.filter(
      (s) => s.sourceLanguage === ESourceLanguage.C,
    );
    const cppDefs = globalDefinitions.filter(
      (s) => s.sourceLanguage === ESourceLanguage.Cpp,
    );
 
    // Issue #967: Only global-scope C-Next symbols can conflict with C/C++ symbols.
    // Scoped symbols (e.g., Touch.read) live in a namespace and don't compete
    // with C's global symbols (e.g., POSIX read()).
    const conflictingCnextDefs = cnextDefs.filter((s) => {
      const tSymbol = s as TSymbol;
      return ScopeUtils.isGlobalScopePath(tSymbol.scopePath);
    });
 
    if (
      conflictingCnextDefs.length > 0 &&
      (cDefs.length > 0 || cppDefs.length > 0)
    ) {
      const conflictingDefs = [...conflictingCnextDefs, ...cDefs, ...cppDefs];
      // #1334: both conflict kinds render the location the same way, through
      // `locationOf` -- these producers used to spell it differently (`LANG
      // (file:line)` here, bare `file:line` below), which is one decision written
      // twice. Only the language ANNOTATION is branch-specific, and it has to stay:
      // the message says the definitions are in multiple languages but not which is
      // which, and for a `.h` the reader cannot tell C from C++ -- the very
      // distinction detectAssemblySyntax points users at this message for.
      //
      // Deduplicated because one C declaration can yield two symbols at one position
      // (`typedef struct {...} helper;` registers both the tag and the alias), which
      // printed the same file:line twice and told the reader nothing.
      const locations = [
        ...new Set(
          conflictingDefs.map(
            (definition) =>
              `${ConflictDetector.locationOf(definition)} (${ConflictDetector.languageName(definition)})`,
          ),
        ),
      ];
 
      return {
        code: "E0425",
        symbolName: conflictingDefs[0].name,
        definitions: conflictingDefs,
        severity: "error",
        sourceFile: conflictingDefs[0].sourceFile,
        line: conflictingDefs[0].span.line,
        // #1318: the symbol's own column. This was hardcoded 0 because no
        // symbol carried one -- the Tier 1 table promised a symbol-level
        // diagnostic could point as precisely as any other, and it could not.
        column: conflictingDefs[0].span.column,
        // The remediation line is INDENTED like the locations. The CLI's error format
        // treats an unindented line as the start of a new diagnostic, so an
        // unindented sentence here is dropped by any consumer that parses stderr --
        // including the test harness, which would capture the locations and silently
        // lose this line (scripts/test-utils.ts, continuation-line branch).
        message: `Symbol conflict: '${conflictingDefs[0].name}' is defined in multiple languages:\n  ${locations.join("\n  ")}\n  Rename the C-Next symbol to resolve.`,
      };
    }
 
    // Multiple definitions in same language (excluding overloads) = ERROR
    const cnextConflict = ConflictDetector.detectCNextDuplicate(
      registry,
      cnextDefs,
    );
    if (cnextConflict) {
      return cnextConflict;
    }
 
    // Same symbol in C and C++ - typically OK (same symbol)
    if (cDefs.length > 0 && cppDefs.length > 0) {
      return null;
    }
 
    return null;
  }
 
  /**
   * Two definitions of the same C-Next symbol in the same scope = ERROR.
   *
   * Issue #817: grouped by scope AND kind — symbols in different scopes do not
   * conflict (`Foo.enabled` and `Bar.enabled` generate distinct C names), and
   * symbols of different kinds do not either (a variable `LED` and a scope `LED`
   * are distinct).
   *
   * Extracted from detectConflict so that method stays under SonarCloud's
   * cognitive-complexity limit; the #1333 scope-reopening branch pushed it over.
   */
  private static detectCNextDuplicate(
    registry: SymbolRegistry | null,
    cnextDefs: TAnySymbol[],
  ): IConflict | null {
    if (cnextDefs.length <= 1) {
      return null;
    }
 
    const byScopeAndKind =
      ConflictDetector.groupCNextSymbolsByScopeAndKind(cnextDefs);
 
    for (const symbols of byScopeAndKind.values()) {
      if (symbols.length <= 1) {
        continue;
      }
 
      // #1333: a scope declaration is not a definition in the sense this rule
      // means. Declaring `scope Lib` a second time REOPENS it and adds members;
      // it does not redefine it -- the model ADR-002:256 described ("one
      // namespace can span files") and ADR-016 now carries forward. Without
      // this, a scope could not be split across files, and could not even be
      // reopened within one file, which defeats the organizational purpose
      // scopes exist for.
      //
      // Members still conflict normally: they are grouped by the scope's own
      // identity, so two `Lib.useIt` definitions collide whichever block they
      // were written in.
      if (symbols[0].kind === "scope") {
        continue;
      }
 
      const locations = symbols.map(ConflictDetector.locationOf);
      // #1285: the symbol's own source-language name. This was built here by
      // hand from `scope.name`, which is the leaf -- at depth two it reported
      // `Inner.tick` for a symbol the author writes as `Outer.Inner.tick`.
      const displayName = symbols[0].cnxScopedName;
      // #1334: when the members belong to a scope, name where that scope is
      // DECLARED as well as where the members are defined. A scope spanning four
      // files is where a duplicate member is hardest to find, and the blocks are
      // exactly what the reader needs to look through.
      //
      // This is also what makes declarationSites observable: without a consumer
      // it would be a write-only field, testable only by unit tests that reach
      // into it -- the shape #1330's review caught as a method with no caller.
      const scopeSites = ConflictDetector.scopeDeclarationNote(
        registry,
        symbols[0],
      );
      return {
        code: "E0425",
        symbolName: displayName,
        definitions: symbols,
        severity: "error",
        // Report at the first offending definition. Each symbol carries its own
        // position, so a member declared in two blocks of a scope spanning four
        // files names the block it actually came from (#1334).
        sourceFile: symbols[0].sourceFile,
        line: symbols[0].span.line,
        // #1318: the symbol's own column, not a hardcoded 0.
        column: symbols[0].span.column,
        message: `Symbol conflict: '${displayName}' is defined multiple times in C-Next:\n  ${locations.join("\n  ")}${scopeSites}`,
      };
    }
 
    return null;
  }
 
  /**
   * Issue #817: Group C-Next symbols by scope name and kind.
   *
   * Symbols in different scopes don't conflict (Foo.enabled vs Bar.enabled
   * generate Foo_enabled and Bar_enabled). Symbols with different kinds also
   * don't conflict (variable LED vs scope LED are distinct).
   *
   * @param symbols C-Next symbols to group (must all be TSymbol)
   * @returns Map from "scopeName:kind" key to array of symbols
   */
  private static groupCNextSymbolsByScopeAndKind(
    symbols: TAnySymbol[],
  ): Map<string, TSymbol[]> {
    const byScopeAndKind = new Map<string, TSymbol[]>();
 
    for (const def of symbols) {
      const tSymbol = def as TSymbol;
      // #1285: key on the scope's own identity, not its leaf name. Two distinct
      // scopes can share a leaf (`Outer.Inner` and `Other.Inner`), and keying on
      // the leaf grouped their members together -- reporting a conflict between
      // symbols that never shared a scope. #1298: the path IS that identity, and
      // is injective for the same reason the C name was.
      const key = `${tSymbol.scopePath}:${tSymbol.kind}`;
      const existing = byScopeAndKind.get(key);
      if (existing) {
        existing.push(tSymbol);
      } else {
        byScopeAndKind.set(key, [tSymbol]);
      }
    }
 
    return byScopeAndKind;
  }
}
 
export default ConflictDetector;