From cfb9db35098f094334fc398bcd9972418e69b313 Mon Sep 17 00:00:00 2001 From: Marcus Ortiz Date: Fri, 5 May 2023 10:27:25 -0700 Subject: [PATCH] Render all attributes for Swift symbol declarations on a single line (#633) A previous change was made so that each attribute was broken onto its own line, but this attempts to collapse all attributes onto their own single line. Example: ```swift // before @discardableResult @objc(bar) func foo() -> Int // after @discardableResult @objc(bar) func foo() -> Int ``` Resolves: rdar://108798969 --- .../PrimaryContent/DeclarationSource.vue | 21 ++- .../PrimaryContent/DeclarationSource.spec.js | 139 ++++++++---------- 2 files changed, 71 insertions(+), 89 deletions(-) diff --git a/src/components/DocumentationTopic/PrimaryContent/DeclarationSource.vue b/src/components/DocumentationTopic/PrimaryContent/DeclarationSource.vue index 41fca540a..04492e3a9 100644 --- a/src/components/DocumentationTopic/PrimaryContent/DeclarationSource.vue +++ b/src/components/DocumentationTopic/PrimaryContent/DeclarationSource.vue @@ -84,7 +84,6 @@ export default { let indentedParams = false; const newTokens = []; let i = 0; - let j = 1; let openParenTokenIndex = null; let openParenCharIndex = null; let closeParenTokenIndex = null; @@ -96,7 +95,8 @@ export default { // keep track of the current token and the next one (if any) const token = tokens[i]; const newToken = { ...token }; - const nextToken = j < tokens.length ? tokens[j] : undefined; + const prevToken = tokens[i - 1]; + const nextToken = tokens[i + 1]; // loop through the token text to look for "(" and ")" characters const tokenLength = (token.text || '').length; @@ -127,15 +127,13 @@ export default { } } - // check if this is a text token following an attribute token - // so we can insert a newline here and split each attribute onto its - // own line - // - // we want to avoid doing this when the attribute is encountered - // in a param clause for attributes like `@escaping` - if (token.kind === TokenKind.text && i > 0 - && tokens[i - 1].kind === TokenKind.attribute - && numUnclosedParens === 0) { + // Find the text following the last attribute preceding the start of a + // declaration by determining if this is the text token in between an + // attribute and a keyword outside of any parameter clause. A newline + // will be added to break these attributes onto their own single line. + if (token.kind === TokenKind.text && numUnclosedParens === 0 + && prevToken && prevToken.kind === TokenKind.attribute + && nextToken && nextToken.kind === TokenKind.keyword) { newToken.text = `${token.text.trimEnd()}\n`; } @@ -150,7 +148,6 @@ export default { newTokens.push(newToken); i += 1; - j += 1; } // if we indented some params, we want to find the opening "(" symbol diff --git a/tests/unit/components/DocumentationTopic/PrimaryContent/DeclarationSource.spec.js b/tests/unit/components/DocumentationTopic/PrimaryContent/DeclarationSource.spec.js index cdfdb1e59..bbabf9a6f 100644 --- a/tests/unit/components/DocumentationTopic/PrimaryContent/DeclarationSource.spec.js +++ b/tests/unit/components/DocumentationTopic/PrimaryContent/DeclarationSource.spec.js @@ -126,6 +126,10 @@ describe('Swift function/initializer formatting', () => { }, }); + const getText = tokens => tokens.wrappers.reduce((txt, token) => ( + `${txt}${token.props('text')}` + ), ''); + it('does not add any whitespace for single-param symbols', () => { // Before: // init(_ foo: Foo) @@ -168,14 +172,8 @@ describe('Swift function/initializer formatting', () => { }, ]; const wrapper = mountWithTokens(tokens); - const tokenComponents = wrapper.findAll(Token); - expect(tokenComponents.length).toBe(tokens.length); - tokens.forEach((token, i) => { - const tokenComponent = tokenComponents.at(i); - expect(tokenComponent.props('kind')).toBe(token.kind); - expect(tokenComponent.props('text')).toBe(token.text); - }); + expect(getText(tokenComponents)).toBe('init(_ foo: Foo)'); }); it('breaks apart each param onto its own line for multi-param symbols', () => { @@ -185,7 +183,7 @@ describe('Swift function/initializer formatting', () => { // After: // func foo( // _ a: A, - // _ b: B, + // _ b: B // ) -> Bar const tokens = [ { @@ -263,22 +261,12 @@ describe('Swift function/initializer formatting', () => { const wrapper = mountWithTokens(tokens); const tokenComponents = wrapper.findAll(Token); - expect(tokenComponents.length).toBe(tokens.length); - - const modifiedTokenIndexes = new Set([3, 9, 15]); - tokens.forEach((token, i) => { - const tokenComponent = tokenComponents.at(i); - expect(tokenComponent.props('kind')).toBe(token.kind); - if (modifiedTokenIndexes.has(i)) { - expect(tokenComponent.props('text')).not.toBe(token.text); - } else { - expect(tokenComponent.props('text')).toBe(token.text); - } - }); - - expect(tokenComponents.at(3).props('text')).toBe('(\n '); - expect(tokenComponents.at(9).props('text')).toBe(',\n '); - expect(tokenComponents.at(15).props('text')).toBe('\n) -> '); + expect(getText(tokenComponents)).toBe( +`func foo( + _ a: A, + _ b: B +) -> Bar`, + ); }); it('breaks apart each param onto its own line for a tuple return type', () => { @@ -288,7 +276,7 @@ describe('Swift function/initializer formatting', () => { // After: // func foo( // _ a: A, - // _ b: B, + // _ b: B // ) -> (A, B) const tokens = [ { @@ -379,34 +367,24 @@ describe('Swift function/initializer formatting', () => { const wrapper = mountWithTokens(tokens); const tokenComponents = wrapper.findAll(Token); - expect(tokenComponents.length).toBe(tokens.length); - - const modifiedTokenIndexes = new Set([3, 9, 15]); - tokens.forEach((token, i) => { - const tokenComponent = tokenComponents.at(i); - expect(tokenComponent.props('kind')).toBe(token.kind); - if (modifiedTokenIndexes.has(i)) { - expect(tokenComponent.props('text')).not.toBe(token.text); - } else { - expect(tokenComponent.props('text')).toBe(token.text); - } - }); - - expect(tokenComponents.at(3).props('text')).toBe('(\n '); - expect(tokenComponents.at(9).props('text')).toBe(',\n '); - expect(tokenComponents.at(15).props('text')).toBe('\n) -> ('); + expect(getText(tokenComponents)).toBe( +`func foo( + _ a: A, + _ b: B +) -> (A, B)`, + ); }); it('breaks apart parameters in functions with generic where clauses', () => { /* eslint-disable max-len */ // Before: - // public func f(t: T, u: U) where T : Sequence, U : Sequence, T.Iterator.Element : Equatable, T.Iterator.Element == U.Iterator.Element + // public func f(t: T, u: U) where U : Sequence, T : Sequence, T.Element : Equatable, U.Element == T.Element // // After: // public func f( // t: T, - // u: U, - // ) where T : Sequence, U : Sequence, T.Iterator.Element : Equatable, T.Iterator.Element == U.Iterator.Element + // u: U + // ) where U : Sequence, T : Sequence, T.Element : Equatable, U.Element == T.Element /* eslint-enable max-len */ const tokens = [ { @@ -567,22 +545,12 @@ describe('Swift function/initializer formatting', () => { const wrapper = mountWithTokens(tokens); const tokenComponents = wrapper.findAll(Token); - expect(tokenComponents.length).toBe(tokens.length); - - const modifiedTokenIndexes = new Set([5, 9, 13]); - tokens.forEach((token, i) => { - const tokenComponent = tokenComponents.at(i); - expect(tokenComponent.props('kind')).toBe(token.kind); - if (modifiedTokenIndexes.has(i)) { - expect(tokenComponent.props('text')).not.toBe(token.text); - } else { - expect(tokenComponent.props('text')).toBe(token.text); - } - }); - - expect(tokenComponents.at(5).props('text')).toBe('(\n '); - expect(tokenComponents.at(9).props('text')).toBe(',\n '); - expect(tokenComponents.at(13).props('text')).toBe('\n) '); + expect(getText(tokenComponents)).toBe( +`public func f( + t: T, + u: U +) where U : Sequence, T : Sequence, U.Element : Equatable, U.Element == T.Element`, + ); }); it('indents parameters using provided/customizable indentation width', () => { @@ -600,7 +568,7 @@ describe('Swift function/initializer formatting', () => { // After: // func foo( // _ a: A, - // _ b: B, + // _ b: B // ) -> Bar const tokens = [ { @@ -678,22 +646,22 @@ describe('Swift function/initializer formatting', () => { const wrapper = mountWithTokens(tokens); const tokenComponents = wrapper.findAll(Token); - expect(tokenComponents.length).toBe(tokens.length); - // should be indented with 2 spaces now instead of the default of 4 spaces - expect(tokenComponents.at(3).props('text')).toBe('(\n '); - expect(tokenComponents.at(9).props('text')).toBe(',\n '); - expect(tokenComponents.at(15).props('text')).toBe('\n) -> '); + expect(getText(tokenComponents)).toBe( +`func foo( + _ a: A, + _ b: B +) -> Bar`, + ); themeSettingsState.theme = originalTheme; }); - it('breaks attributes onto their own lines', () => { + it('breaks attributes onto their own line', () => { // Before: // @discardableResult @objc(baz) func foobarbaz() -> Int // // After: - // @discardableResult - // @objc(baz) + // @discardableResult @objc(baz) // func foobarbaz() -> Int const tokens = [ { @@ -737,9 +705,10 @@ describe('Swift function/initializer formatting', () => { const wrapper = mountWithTokens(tokens); const tokenComponents = wrapper.findAll(Token); - expect(tokenComponents.length).toBe(tokens.length); - expect(tokenComponents.at(1).props('text')).toBe('\n'); - expect(tokenComponents.at(3).props('text')).toBe('(baz)\n'); + expect(getText(tokenComponents)).toBe( +`@discardableResult @objc(baz) +func foobarbaz() -> Int`, + ); }); it('does not add newlines to attributes within param clause', () => { @@ -782,11 +751,27 @@ describe('Swift function/initializer formatting', () => { text: ')', }, ]; - const wrapper = mountWithTokens(tokens); + let wrapper = mountWithTokens(tokens); - const tokenComponents = wrapper.findAll(Token); - expect(tokenComponents.length).toBe(tokens.length); - expect(tokenComponents.at(6).props('text')).toBe(tokens[6].text); - expect(tokenComponents.at(7).props('text')).toBe(tokens[7].text); + let tokenComponents = wrapper.findAll(Token); + expect(getText(tokenComponents)).toBe('func foo(bar: @escaping () -> ())'); + + // @discardableResult func foo(bar: @escaping () -> ()) -> Int + wrapper = mountWithTokens([ + { kind: 'attribute', text: '@discardableResult' }, + { kind: 'text', text: ' ' }, + ...tokens, + { kind: 'text', text: ' -> ' }, + { + kind: 'typeIdentifier', + identifier: 'doc://com.example/documentation/blah/int', + text: 'Int', + }, + ]); + tokenComponents = wrapper.findAll(Token); + expect(getText(tokenComponents)).toBe( +`@discardableResult +func foo(bar: @escaping () -> ()) -> Int`, + ); }); });