diff --git a/packages/react-native/scripts/spm/__tests__/inject-spm-xcodeproj-test.js b/packages/react-native/scripts/spm/__tests__/inject-spm-xcodeproj-test.js index 89393a28dc0..554b1030cdf 100644 --- a/packages/react-native/scripts/spm/__tests__/inject-spm-xcodeproj-test.js +++ b/packages/react-native/scripts/spm/__tests__/inject-spm-xcodeproj-test.js @@ -29,6 +29,16 @@ const PODS = PLAIN.replace( 'AA0000000000000000000901 /* Debug */ = {\n\t\t\tisa = XCBuildConfiguration;\n\t\t\tbaseConfigurationReference = BB0000000000000000000001 /* Pods-MyApp.debug.xcconfig */;\n\t\t\tbuildSettings = {', ); +// Derive a variant whose app-target configs already carry HEADER_SEARCH_PATHS, +// set to any valid pbxproj value: a plain scalar (which injection promotes to an +// array) or an array injection appends to. +function withHeaderSearchPaths(value) { + return PLAIN.replaceAll( + 'PRODUCT_BUNDLE_IDENTIFIER = com.example.MyApp;', + `HEADER_SEARCH_PATHS = ${value};\n\t\t\t\tPRODUCT_BUNDLE_IDENTIFIER = com.example.MyApp;`, + ); +} + const RN_PATH = '../node_modules/react-native'; // Absolute, mirroring resolveHermesCliPathSetting (a `..`-relative path through @@ -227,6 +237,50 @@ describe('injectSpmIntoPbxproj — Tier 2 (build settings + phase)', () => { expect(syncIdx).toBeLessThan(sourcesIdx); }); + it.each([ + [ + '"$(inherited)"', + ['"$(inherited)"', '"$(SRCROOT)/build/generated/autolinking/headers"'], + ], + [ + '"$(inherited) $(SRCROOT)/vendor/include"', + [ + '"$(inherited)"', + '"$(inherited) $(SRCROOT)/vendor/include"', + '"$(SRCROOT)/build/generated/autolinking/headers"', + ], + ], + ])( + 'promotes a pre-existing HEADER_SEARCH_PATHS scalar (%s) to an array, keeping its value and one $(inherited)', + (scalar, expectedMembers) => { + const {text} = inject(withHeaderSearchPaths(scalar)); + const arrays = [ + ...text.matchAll(/HEADER_SEARCH_PATHS = \(\n([\s\S]*?)\t+\);/g), + ].map(m => + m[1] + .split('\n') + .map(line => line.trim().replace(/,$/, '')) + .filter(member => member.length > 0), + ); + // Both app-target configs (Debug + Release). + expect(arrays).toEqual([expectedMembers, expectedMembers]); + }, + ); + + it('appends to a pre-existing ONE-LINE HEADER_SEARCH_PATHS array in place', () => { + const {text} = inject(withHeaderSearchPaths('("$(inherited)", )')); + expect(isBalanced(text)).toBe(true); + const arrays = [ + ...text.matchAll(/HEADER_SEARCH_PATHS = \(([^\n]*)\);/g), + ].map(m => m[1]); + // Both app-target configs, each keeping the one-line shape it was written in. + expect(arrays).toEqual( + Array(2).fill( + '"$(inherited)", "$(SRCROOT)/build/generated/autolinking/headers", ', + ), + ); + }); + it('adds one generated embed phase immediately after Frameworks', () => { const {text} = inject(PLAIN); expect(text).not.toContain('Fix SPM Embedded Flavor'); diff --git a/packages/react-native/scripts/spm/__tests__/remove-spm-injection-test.js b/packages/react-native/scripts/spm/__tests__/remove-spm-injection-test.js index 4fe3d342fdc..4a5016babd9 100644 --- a/packages/react-native/scripts/spm/__tests__/remove-spm-injection-test.js +++ b/packages/react-native/scripts/spm/__tests__/remove-spm-injection-test.js @@ -34,15 +34,44 @@ afterEach(() => { scaffoldedAppRoots = []; }); +// Pre-existing values for an injected array setting (HEADER_SEARCH_PATHS), +// seeded into both app-target configs. The plain fixture has none, so it only +// ever exercises the create-from-absent path; deinit must restore each of +// these forms — a plain scalar is ordinary, valid pbxproj. +const PRE_EXISTING_HEADER_SEARCH_PATHS = { + 'a bare $(inherited) scalar': '"$(inherited)"', + 'a scalar with real content': '"$(inherited) $(SRCROOT)/vendor/include"', + 'an array': '(\n\t\t\t\t"$(inherited)",\n\t\t\t)', + // What hand edits and other generators (XcodeGen, Tuist) write. + 'a one-line array': '("$(inherited)", )', +}; + +// Seed a whole `KEY = value;` field (comments and stray whitespace included) +// into both app-target configs. +function withSetting(field /*: string */) { + return PLAIN.replaceAll( + 'PRODUCT_BUNDLE_IDENTIFIER = com.example.MyApp;', + `${field}\n\t\t\t\tPRODUCT_BUNDLE_IDENTIFIER = com.example.MyApp;`, + ); +} + +function withHeaderSearchPaths(value /*: string */) { + return withSetting(`HEADER_SEARCH_PATHS = ${value};`); +} + // Build a throwaway app dir: /MyApp.xcodeproj/project.pbxproj seeded with // the plain (SPM-only) fixture, and a node_modules/react-native sibling so the // relative reactNativePath resolves. -function scaffoldApp() { +function scaffoldApp(pbxproj = PLAIN) { const appRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'spm-deinit-')); scaffoldedAppRoots.push(appRoot); const xcodeprojPath = path.join(appRoot, 'MyApp.xcodeproj'); fs.mkdirSync(xcodeprojPath, {recursive: true}); - fs.writeFileSync(path.join(xcodeprojPath, 'project.pbxproj'), PLAIN, 'utf8'); + fs.writeFileSync( + path.join(xcodeprojPath, 'project.pbxproj'), + pbxproj, + 'utf8', + ); const rnRoot = path.join(appRoot, 'node_modules', 'react-native'); fs.mkdirSync(rnRoot, {recursive: true}); const artifactRoot = path.join(appRoot, 'build', 'xcframeworks'); @@ -189,6 +218,144 @@ describe('removeSpmInjection — the surgical inverse of add', () => { }); }); +describe.each(Object.entries(PRE_EXISTING_HEADER_SEARCH_PATHS))( + 'removeSpmInjection with HEADER_SEARCH_PATHS already set to %s', + (_label, value) => { + it('restores the pre-existing value byte-for-byte', () => { + const {appRoot, xcodeprojPath, rnRoot} = scaffoldApp( + withHeaderSearchPaths(value), + ); + const before = pbxprojOf(xcodeprojPath); + + injectSpmIntoExistingXcodeproj({ + appRoot, + reactNativeRoot: rnRoot, + xcodeprojPath, + }); + expect(pbxprojOf(xcodeprojPath)).not.toBe(before); + + expect(removeSpmInjection({appRoot, xcodeprojPath}).status).toBe( + 'removed', + ); + expect(pbxprojOf(xcodeprojPath)).toBe(before); + }); + + it('re-syncing is byte-for-byte identical', () => { + const {appRoot, xcodeprojPath, rnRoot} = scaffoldApp( + withHeaderSearchPaths(value), + ); + injectSpmIntoExistingXcodeproj({ + appRoot, + reactNativeRoot: rnRoot, + xcodeprojPath, + }); + const first = pbxprojOf(xcodeprojPath); + injectSpmIntoExistingXcodeproj({ + appRoot, + reactNativeRoot: rnRoot, + xcodeprojPath, + }); + expect(pbxprojOf(xcodeprojPath)).toBe(first); + }); + }, +); + +// findField's token for a BARE scalar ends AT the `;`, so it includes any +// whitespace before it. Deinit must put those bytes back exactly, not a +// tidied-up version of them. +describe.each([ + 'HEADER_SEARCH_PATHS = $(inherited) ; /* note */', + 'HEADER_SEARCH_PATHS = ;', +])('removeSpmInjection with the untrimmed scalar `%s`', field => { + it('restores it byte-for-byte', () => { + const {appRoot, xcodeprojPath, rnRoot} = scaffoldApp(withSetting(field)); + const before = pbxprojOf(xcodeprojPath); + + injectSpmIntoExistingXcodeproj({ + appRoot, + reactNativeRoot: rnRoot, + xcodeprojPath, + }); + expect(pbxprojOf(xcodeprojPath)).not.toBe(before); + + expect(removeSpmInjection({appRoot, xcodeprojPath}).status).toBe('removed'); + expect(pbxprojOf(xcodeprojPath)).toBe(before); + }); +}); + +describe('a scalar array setting injection has nothing to add to', () => { + const SCALAR = 'FRAMEWORK_SEARCH_PATHS = "$(inherited)";'; + const EDITED = 'FRAMEWORK_SEARCH_PATHS = "$(inherited) $(SRCROOT)/Vendor";'; + + // The fixture's flavored-frameworks manifest is empty, so + // FRAMEWORK_SEARCH_PATHS is injected with no values at all. + it('is left untouched, unrecorded, and survives a later user edit', () => { + const {appRoot, xcodeprojPath, rnRoot} = scaffoldApp(withSetting(SCALAR)); + + injectSpmIntoExistingXcodeproj({ + appRoot, + reactNativeRoot: rnRoot, + xcodeprojPath, + }); + + const injected = pbxprojOf(xcodeprojPath); + expect(injected).toContain(SCALAR); + expect(injected).not.toMatch(/FRAMEWORK_SEARCH_PATHS = \(/); + for (const change of readMarker(xcodeprojPath).buildSettingChanges) { + expect(change.promotedArrayScalars ?? {}).not.toHaveProperty( + 'FRAMEWORK_SEARCH_PATHS', + ); + } + + fs.writeFileSync( + path.join(xcodeprojPath, 'project.pbxproj'), + injected.replaceAll(SCALAR, EDITED), + 'utf8', + ); + removeSpmInjection({appRoot, xcodeprojPath}); + + const after = pbxprojOf(xcodeprojPath); + expect(after).toContain(EDITED); + expect(after).not.toContain(SCALAR); + }); +}); + +describe('a promoted array setting the user deleted after add', () => { + const SCALAR = '"$(inherited) $(SRCROOT)/vendor/include"'; + + it('is not resurrected by deinit', () => { + const {appRoot, xcodeprojPath, rnRoot} = scaffoldApp( + withHeaderSearchPaths(SCALAR), + ); + const before = pbxprojOf(xcodeprojPath); + + injectSpmIntoExistingXcodeproj({ + appRoot, + reactNativeRoot: rnRoot, + xcodeprojPath, + }); + + const deleted = pbxprojOf(xcodeprojPath).replace( + /\n\t+HEADER_SEARCH_PATHS = \(\n[\s\S]*?\n\t+\);/g, + '', + ); + expect(deleted).not.toContain('HEADER_SEARCH_PATHS'); + fs.writeFileSync( + path.join(xcodeprojPath, 'project.pbxproj'), + deleted, + 'utf8', + ); + + removeSpmInjection({appRoot, xcodeprojPath}); + + // Everything else is back to its pre-injection bytes; only the setting the + // user deleted stays gone. + expect(pbxprojOf(xcodeprojPath)).toBe( + before.replaceAll(`\n\t\t\t\tHEADER_SEARCH_PATHS = ${SCALAR};`, ''), + ); + }); +}); + describe('generated-sources reconciliation on update', () => { it('removes exactly the UUIDs of an entry dropped from the manifest, keeping the rest', () => { const {appRoot, xcodeprojPath, rnRoot} = scaffoldApp(); diff --git a/packages/react-native/scripts/spm/__tests__/spm-pbxproj-test.js b/packages/react-native/scripts/spm/__tests__/spm-pbxproj-test.js index 2ac7297d4ce..0ce4b33a955 100644 --- a/packages/react-native/scripts/spm/__tests__/spm-pbxproj-test.js +++ b/packages/react-native/scripts/spm/__tests__/spm-pbxproj-test.js @@ -41,6 +41,19 @@ const PLAIN_PBXPROJ = fs.readFileSync( 'utf8', ); +// The app target's Debug buildSettings dict, as a body range. +function targetDebugDict(text) { + const cfg = findObjectByUuid(text, 'AA0000000000000000000901'); + const bs = findField(text, cfg, 'buildSettings'); + return {uuid: 'x', bodyOpen: bs.valueStart, bodyClose: bs.tokenEnd - 1}; +} + +// Delimiter balance, checked with the module's own quote-aware scanner: the +// outermost `{` must close on the file's last `}`. +function isBalanced(text) { + return scanToClose(text, text.indexOf('{')) === text.lastIndexOf('}'); +} + // --------------------------------------------------------------------------- // generateUUID // --------------------------------------------------------------------------- @@ -161,12 +174,6 @@ describe('addArrayMembers', () => { }); describe('addArrayStringValues', () => { - function targetDebugDict(text) { - const cfg = findObjectByUuid(text, 'AA0000000000000000000901'); - const bs = findField(text, cfg, 'buildSettings'); - return {uuid: 'x', bodyOpen: bs.valueStart, bodyClose: bs.tokenEnd - 1}; - } - it('creates an array seeded with $(inherited)', () => { const out = addArrayStringValues( PLAIN_PBXPROJ, @@ -195,6 +202,50 @@ describe('addArrayStringValues', () => { expect(out).toContain('"-ObjC"'); }); + // Xcode writes the seed quoted, but the unquoted form is just as valid and + // appears in hand-edited projects. Both are the same value to the build + // system, so neither may be re-emitted alongside the seed. + it.each(['"$(inherited)"', '$(inherited)'])( + 'promotes a bare %s scalar without emitting the seed twice', + priorValue => { + const scalar = PLAIN_PBXPROJ.replace( + 'PRODUCT_NAME = "$(TARGET_NAME)";', + `OTHER_LDFLAGS = ${priorValue}; PRODUCT_NAME = "$(TARGET_NAME)";`, + ); + const out = addArrayStringValues( + scalar, + targetDebugDict(scalar), + 'OTHER_LDFLAGS', + ['"-ObjC"'], + ); + const members = /OTHER_LDFLAGS = \(\n([\s\S]*?)\t+\);/ + .exec(out)[1] + .split('\n') + .map(line => line.trim().replace(/,$/, '')) + .filter(member => member.length > 0); + // The scalar's value IS the seed the array is created with. + expect(members).toEqual(['"$(inherited)"', '"-ObjC"']); + }, + ); + + it('promotes an empty scalar without emitting a bare `,` member', () => { + const scalar = PLAIN_PBXPROJ.replace( + 'PRODUCT_NAME = "$(TARGET_NAME)";', + 'OTHER_LDFLAGS = ; PRODUCT_NAME = "$(TARGET_NAME)";', + ); + const out = addArrayStringValues( + scalar, + targetDebugDict(scalar), + 'OTHER_LDFLAGS', + ['"-ObjC"'], + ); + const block = /OTHER_LDFLAGS = \(\n([\s\S]*?)\t+\);/.exec(out)[1]; + // Asserted on the raw block: a member list filtered for emptiness (as the + // test above does) would hide the malformed element this guards against. + expect(block.split('\n').filter(line => /^\s*,$/.test(line))).toEqual([]); + expect(block).toContain('"-ObjC"'); + }); + it('dedups by EXACT token, not substring (adds "-ObjC" even when "-ObjCFoo" is present)', () => { const withArray = PLAIN_PBXPROJ.replace( 'PRODUCT_NAME = "$(TARGET_NAME)";', @@ -226,6 +277,102 @@ describe('addArrayStringValues', () => { }); }); +// Xcode writes array build settings multi-line, but hand-edited projects and +// other generators (XcodeGen, Tuist) emit compact one-line ones. Members must +// land INSIDE the array whatever its shape, and `deinit` must be able to take +// them back out again — hence the byte-identical add→remove round trip. +describe('array build settings of every written shape', () => { + const NEW = '"/new"'; + + // Seed `OTHER_LDFLAGS = ;` into the app target's Debug config. + function withValue(value) { + return PLAIN_PBXPROJ.replace( + '\t\t\t\tPRODUCT_NAME = "$(TARGET_NAME)";', + `\t\t\t\tOTHER_LDFLAGS = ${value};\n\t\t\t\tPRODUCT_NAME = "$(TARGET_NAME)";`, + ); + } + + function add(text, values) { + return addArrayStringValues( + text, + targetDebugDict(text), + 'OTHER_LDFLAGS', + values, + ); + } + + function remove(text, values) { + return removeArrayStringValues( + text, + targetDebugDict(text), + 'OTHER_LDFLAGS', + values, + ); + } + + describe.each([ + ['an empty array', '()'], + ['a lone member with no trailing comma', '("/a")'], + ['a trailing comma and space', '("/a", )'], + ['no space after the comma', '("/a","/b")'], + ['a space after the comma', '("/a", "/b")'], + ['a member whose quotes hold a comma and parens', '("$(FOO(x)),weird")'], + ['the multi-line shape Xcode writes', '(\n\t\t\t\t\t"/a",\n\t\t\t\t)'], + ])('%s', (_label, shape) => { + const input = withValue(shape); + + it('adds the value inside the array, leaving the file balanced', () => { + const out = add(input, [NEW]); + const field = findField(out, targetDebugDict(out), 'OTHER_LDFLAGS'); + expect(field.value.trimStart().startsWith('(')).toBe(true); + expect(field.value).toContain(NEW); + // Nothing was spliced ahead of the field — i.e. outside the array. + expect(out.slice(0, field.matchStart)).toBe( + input.slice(0, field.matchStart), + ); + expect(isBalanced(out)).toBe(true); + }); + + it.each([[[NEW]], [[NEW, '"/new2"']]])( + 'remove undoes add of %j byte-for-byte', + values => { + const added = add(input, values); + expect(added).not.toBe(input); + expect(remove(added, values)).toBe(input); + }, + ); + }); + + it.each([ + ['a value that is not there', '("/a", )', '"/zzz"'], + ['a member stripped of its quotes', '("$(inherited)", )', '$(inherited)'], + ])('removes nothing when asked for %s', (_label, shape, value) => { + const input = withValue(shape); + expect(remove(input, [value])).toBe(input); + }); + + it('is a no-op when a one-line array already holds the value', () => { + const input = withValue(`(${NEW})`); + expect(add(input, [NEW])).toBe(input); + }); + + it('dedupes a member whose quotes hold a comma', () => { + const weird = '"$(FOO(x)),weird"'; + const input = withValue(`(${weird}, )`); + expect(add(input, [weird])).toBe(input); + }); + + it('splices a multi-line array on its own line, before the closing `)`', () => { + const input = withValue('(\n\t\t\t\t\t"/a",\n\t\t\t\t)'); + expect(add(input, [NEW])).toBe( + input.replace( + '\t\t\t\t\t"/a",\n', + `\t\t\t\t\t"/a",\n\t\t\t\t\t${NEW},\n`, + ), + ); + }); +}); + describe('ensureScalarField', () => { it('adds a scalar only when absent', () => { const project = findProjectObject(PLAIN_PBXPROJ); diff --git a/packages/react-native/scripts/spm/generate-spm-xcodeproj.js b/packages/react-native/scripts/spm/generate-spm-xcodeproj.js index e75af429ce2..ead38f33f3f 100644 --- a/packages/react-native/scripts/spm/generate-spm-xcodeproj.js +++ b/packages/react-native/scripts/spm/generate-spm-xcodeproj.js @@ -136,6 +136,11 @@ type BuildSettingChange = { // value), e.g. a ${PODS_ROOT}-anchored REACT_NATIVE_PATH that dangles once // CocoaPods is deintegrated. Deinit restores the original. replacedScalars?: {[string]: string}, + // Array settings that existed as a SCALAR and were promoted to a `( … )` + // array (key → the pre-injection raw value text, quotes included). Deinit + // restores that value verbatim; removing the injected members would leave + // the promoted array and its `"$(inherited)"` seed behind. + promotedArrayScalars?: {[string]: string}, }; // A plugin-contributed source, normalized for pbxproj emission. `path` is // SRCROOT-relative when under the app root, else absolute; `sourceTree` is the @@ -1492,6 +1497,7 @@ function mergeReactBuildSettings( }; const createdArrayKeys /*: Array */ = []; const appendedArrayValues /*: {[string]: Array} */ = {}; + const promotedArrayScalars /*: {[string]: string} */ = {}; const createdScalars /*: Array */ = []; const arraySettings = [ ...INJECTED_ARRAY_SETTINGS, @@ -1503,15 +1509,32 @@ function mergeReactBuildSettings( continue; } const existing = findField(text, d, key); + // Non-null only for a scalar addArrayStringValues would promote to an array + // (same array-vs-scalar test it uses). Kept RAW: findField's token for a + // bare scalar ends at the `;`, so it carries any whitespace before it, and + // deinit has to write those bytes back verbatim. + const priorScalar = + existing != null && !existing.value.trimStart().startsWith('(') + ? existing.value + : null; if (existing == null) { createdArrayKeys.push(key); - } else { + } else if (priorScalar == null) { const fresh = values.filter(v => !existing.value.includes(v)); if (fresh.length > 0) { appendedArrayValues[key] = fresh; } } + const beforeAdd = text; text = addArrayStringValues(text, d, key, values); + // Record only a promotion that actually happened: addArrayStringValues + // no-ops when `values` is empty or every value is already a member, and a + // recorded-but-untouched field would have deinit clobber whatever the user + // has there by then. Restoring the scalar subsumes removing the injected + // members, so the two records stay mutually exclusive per key. + if (priorScalar != null && text !== beforeAdd) { + promotedArrayScalars[key] = priorScalar; + } } const replacedScalars /*: {[string]: string} */ = {}; for (const {key, value} of scalars) { @@ -1570,6 +1593,9 @@ function mergeReactBuildSettings( appendedArrayValues, createdScalars, replacedScalars, + ...(Object.keys(promotedArrayScalars).length > 0 + ? {promotedArrayScalars} + : {}), }, }; } @@ -2097,6 +2123,25 @@ function removeRecordedBuildSettings( ); } } + const promotedArrayScalars /*: {[string]: string} */ = + change.promotedArrayScalars ?? {}; + for (const key of Object.keys(promotedArrayScalars)) { + const current = dict(); + const originalValue = promotedArrayScalars[key]; + // A field that is gone was deleted by the user after injection; restoring + // it would resurrect it, at the top of the dict, matching neither state. + if ( + current != null && + typeof originalValue === 'string' && + findField(text, current, key) != null + ) { + // Rewriting the whole value is what makes the promotion reversible at + // all — its members and its `"$(inherited)"` seed are indistinguishable + // from the user's own once folded together. The tradeoff: members the + // user hand-added to the promoted array afterwards are discarded. + text = setScalarField(text, current, key, originalValue); + } + } for (const key of change.createdArrayKeys ?? []) { const current = dict(); if (current != null) { diff --git a/packages/react-native/scripts/spm/spm-pbxproj.js b/packages/react-native/scripts/spm/spm-pbxproj.js index 56f27e73ba8..c27d82f6eaa 100644 --- a/packages/react-native/scripts/spm/spm-pbxproj.js +++ b/packages/react-native/scripts/spm/spm-pbxproj.js @@ -367,6 +367,38 @@ function addArrayMembers( return text.slice(0, obj.bodyOpen + 1) + block + text.slice(obj.bodyOpen + 1); } +/** + * Indices of the `,` separators at an array's top level — a comma inside a + * quoted member (`"$(FOO),weird"`) separates nothing. + */ +function topLevelCommas(inner /*: string */) /*: Array */ { + const out = []; + for (let i = 0; i < inner.length; i++) { + if (inner[i] === '"') { + i = scanString(inner, i); + } else if (inner[i] === ',') { + out.push(i); + } + } + return out; +} + +/** + * The members of an array's inner text (what sits between its parens), trimmed + * and with empty slots — e.g. the one a trailing comma leaves — dropped. A bare + * scalar value parses as its own single member. + */ +function arrayMembers(inner /*: string */) /*: Array */ { + const members = []; + let start = 0; + for (const comma of topLevelCommas(inner)) { + members.push(inner.slice(start, comma)); + start = comma + 1; + } + members.push(inner.slice(start)); + return members.map(m => m.trim()).filter(m => m !== ''); +} + /** * Append raw string values to a `( … )` array build-setting (e.g. * OTHER_LDFLAGS), deduping by exact token. Creates the setting seeded with @@ -385,31 +417,57 @@ function addArrayStringValues( const field = findField(text, obj, key); if (field != null) { + const isArray = field.value.trimStart().startsWith('('); + const openParen = isArray + ? field.valueStart + field.value.indexOf('(') + : -1; + const closeParen = isArray ? scanToClose(text, openParen) : -1; + const inner = isArray ? text.slice(openParen + 1, closeParen) : field.value; // Dedup by EXACT existing member, not substring — a substring check would // treat `"-ObjC"` as already present when only `"-ObjCFoo"` is there (and - // vice-versa). Parse the current members (array `( … )` or bare scalar). - const existingMembers = new Set( - field.value - .replace(/^\s*\(/, '') - .replace(/\)\s*$/, '') - .split(',') - .map(s => s.trim()) - .filter(s => s.length > 0), - ); + // vice-versa). + const existingMembers = new Set(arrayMembers(inner)); const fresh = values.filter(v => !existingMembers.has(v)); if (fresh.length === 0) { return text; } - if (field.value.trimStart().startsWith('(')) { - // Existing array — splice fresh members before the closing `)`. - const lineStart = text.lastIndexOf('\n', field.tokenEnd - 1) + 1; - const lines = fresh.map(v => `${memberIndent}${v},\n`).join(''); - return text.slice(0, lineStart) + lines + text.slice(lineStart); + if (isArray) { + if (inner.includes('\n')) { + // Multi-line array — one member per line, before the closing `)`. + const lineStart = text.lastIndexOf('\n', closeParen) + 1; + const lines = fresh.map(v => `${memberIndent}${v},\n`).join(''); + return text.slice(0, lineStart) + lines + text.slice(lineStart); + } + // One-line array (hand-edited projects, XcodeGen, Tuist) — splice the + // members in ahead of the `)`, in the separator style already there. + // Reformatting it multi-line instead would have to record the old shape + // to stay reversible on deinit. + const commas = topLevelCommas(inner); + const gapMatch = + commas.length > 0 ? /^[\t ]*/.exec(inner.slice(commas[0] + 1)) : null; + const gap = gapMatch != null ? gapMatch[0] : ' '; + const core = inner.replace(/[\t ]+$/, ''); + const joined = fresh.join(`,${gap}`); + const insertion = + core === '' + ? joined + : core.endsWith(',') + ? `${gap}${joined},` + : `,${gap}${joined}`; + const at = openParen + 1 + core.length; + return text.slice(0, at) + insertion + text.slice(at); } - // Existing scalar — promote to an array preserving the prior value. + // Existing scalar — promote to an array preserving the prior value. Skip it + // when it IS the `"$(inherited)"` the array is seeded with (emitted twice), + // or when it is empty (a bare `,` is not a valid plist element). The seed + // test unquotes first: pbxproj accepts `$(inherited)` bare, and Xcode's own + // quoted form is the same value to the build system. + const prior = field.value.trim(); + const carriesPrior = + prior !== '' && prior.replace(/^"(.*)"$/s, '$1') !== '$(inherited)'; const replacement = arrayBlock([ '"$(inherited)"', - field.value.trim(), + ...(carriesPrior ? [prior] : []), ...fresh, ]); return ( @@ -464,7 +522,12 @@ function setScalarField( // --------------------------------------------------------------------------- // Surgical removal — the inverse of the additive helpers above. `deinit` uses // these to undo exactly what injection added, leaving every other byte (incl. -// user edits made after injection) untouched. All are pure string transforms. +// user edits made after injection) untouched. The exception is a scalar that +// injection promoted to an array: reversing that rewrites the whole field (see +// removeRecordedBuildSettings), so members added to it afterwards are lost. +// That is not deinit-only — every re-sync reverts from the recorded baseline +// before re-injecting, so an `spm update` discards them just the same. +// All are pure string transforms. // --------------------------------------------------------------------------- /** @@ -525,6 +588,33 @@ function removeField( return text.slice(0, f.matchStart) + text.slice(f.tokenEnd + 1); } +/** + * Drop one member from an array field's value text. The patterns mirror the + * forms addArrayStringValues inserts, tried in the order that makes the span + * removed exactly the span it added: a line of its own (multi-line array), then + * after a comma, before a comma, or alone ahead of the `)` (the one-line + * shapes). Each is anchored on a delimiter, so a value that is merely a prefix + * of a longer member is never mistaken for it. + */ +function removeArrayMember( + region /*: string */, + value /*: string */, +) /*: string */ { + const v = escapeRegExp(value); + for (const pattern of [ + `\\n[\\t ]*${v},`, + `,[\\t ]*${v}(?=[\\t ]*[,)])`, + `[\\t ]*${v},[\\t ]*`, + `[\\t ]*${v}(?=[\\t ]*\\))`, + ]) { + const shorter = region.replace(new RegExp(pattern), ''); + if (shorter !== region) { + return shorter; + } + } + return region; +} + /** * Remove specific raw string members from an existing `( … )` array field * (inverse of addArrayStringValues' append branch). Leaves the field and any @@ -542,7 +632,7 @@ function removeArrayStringValues( } let region = text.slice(f.valueStart, f.tokenEnd); for (const val of values) { - region = region.replace(new RegExp(`\\n[\\t ]*${escapeRegExp(val)},`), ''); + region = removeArrayMember(region, val); } return text.slice(0, f.valueStart) + region + text.slice(f.tokenEnd); }