Harden snippet session against null decorations and selection/placeholder cardinality drift

[dmitrivMS]

Reconciles four telemetry-reported snippet crashes into a single set of defensive guards in OneSnippet / SnippetSession. The underlying root cause for some of these is broader (lack of editor transactions / re-entrant model edits during changeDecorations), which is out of scope here — this change just protects the snippet code without hiding errors or hacking around the model.

Closes #233164
Closes #233163
Closes #196664
Closes #193172

Changes

OneSnippet.move()

• Capture _editor.getModel() into a local model once (instead of repeated this._editor.getModel() calls).
• Null-guard _placeholderDecorations.get(...) and model.getDecorationRange(id) in:
  • the placeholder transform path,
  • the active-placeholder selection path,
  • the enclosing-placeholder loop.
• A missing decoration is silently skipped instead of dereferencing null/undefined.

OneSnippet._hasPlaceholderBeenCollapsed()

• Capture model into a local.
• Treat a missing decoration range on a non-empty placeholder as collapsed (matches the original range.isEmpty() && marker.toString().length > 0 semantics, but tolerant of null).

SnippetSession.merge()

• Add OneSnippet.activePlaceholderCount getter.
• Add a cardinality precondition: only run structural merge and follow-on _move(undefined) when snippets.length === sum(activePlaceholderCount). When cursor normalization or external selection changes collapse mirrored placeholder selections (e.g. $1$1 at the same position), we now skip the structural merge instead of crashing on others.shift() returning undefined. Text edits still apply.

SnippetSession.next() / prev()

• No-op when _move() returns no selections, instead of crashing on newSelections[0].

Tests

Added two regression tests in snippetSession.test.ts:

• 'snippets, next does not throw when placeholder decorations are missing' — covers the null-decoration crashes (Cannot read properties of null (reading 'startLineNumber') #233164, Cannot read properties of null (reading 'startLineNumber') #196664). Inserts a snippet, removes its decorations, calls session.next() and asserts it doesn't throw.
• 'snippets, merge does not throw when placeholder occurrences collapse to same position' — covers the merge-collapse crashes (Cannot read properties of undefined (reading 'e') #233163, Cannot read properties of undefined (reading 'e') #193172). Inserts $1$1$0 (cursor normalization collapses two empty placeholder cursors into one), then calls session.merge('${1:nested}$0') and asserts it doesn't throw with correct document content afterward.

All 40 tests in snippetSession.test.ts and the surrounding 38 in the snippet test directory pass.

Manual repro (merge-collapse class)

User snippets:

{
  "test": { "prefix": "test", "body": "$1$1$0" },
  "nest": { "prefix": "nest", "body": "${1:nested}$0" }
}

1. Type test, accept the suggestion. Buffer is empty, one cursor at column 1 (the two $1 cursors collapsed via normalization).
2. Without leaving the snippet session, type nest and accept the second snippet from the suggestion list.

Pre-fix: TypeError: Cannot read properties of undefined (reading '_offset') logged to the Window output channel; buffer/navigation left in inconsistent state.

Post-fix: No crash. Text edit is applied; structural merge is skipped because the cardinality precondition fails.

The null-decoration class (#233164, #196664) is the result of re-entrant model edits during snippet navigation (e.g. format-on-type / extension edits firing inside changeDecorations) and is not reliably hand-reproducible. Coverage is via the unit test.

Notes on rejected approaches

• An earlier draft (Fix crash in snippet merge when multiple placeholder occurrences collapse to same position #315148) handled the merge-collapse case by deleting the excess placeholder decoration and calling _snippet.replace(placeholder, []) mid-loop. That mutates the snippet model during merge — exactly the kind of hack we want to avoid. The cardinality precondition fixes the same crash without touching the snippet model mid-merge.
• An earlier draft (Guard snippet navigation against stale placeholder decorations #315153) added a "fall back to current editor selections" path inside move() when no valid snippet selection could be produced. That hides errors rather than just preventing the crash; replaced with the simpler "no selections → no-op" guard in next() / prev().

[Copilot]

Pull request overview

This PR hardens snippet navigation and snippet merging in the editor against stale/missing placeholder decorations and against selection/placeholder-count mismatches (e.g., when cursor normalization collapses multiple empty placeholder occurrences into fewer selections), preventing several telemetry-reported crashes in OneSnippet/SnippetSession.

Changes:

• Add null/undefined guards in OneSnippet.move() and _hasPlaceholderBeenCollapsed() for missing decoration IDs and missing decoration ranges, skipping invalid placeholders instead of dereferencing.
• Add OneSnippet.activePlaceholderCount and use it in SnippetSession.merge() to only perform structural merges when the number of nested snippets matches the number of active placeholder occurrences.
• Make SnippetSession.next()/prev() resilient to _move() returning no selections (no-op instead of indexing newSelections[0]).

Show a summary per file

File	Description
src/vs/editor/contrib/snippet/browser/snippetSession.ts	Adds defensive guards for missing decorations/ranges, adds merge cardinality precondition, and avoids crashes when navigation yields no selections.
src/vs/editor/contrib/snippet/test/browser/snippetSession.test.ts	Adds regression tests covering missing-decoration navigation and merge when placeholder occurrences collapse to the same position.

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 0