Skip to content

Render all attributes for Swift symbol declarations on a single line #633

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 5, 2023
Merged

Render all attributes for Swift symbol declarations on a single line #633

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0f24e40
Render all attributes on a single line.
mportiz08 May 2, 2023
c8d6fdb
Refactor `DeclarationSource` formatting specs.
mportiz08 May 2, 2023
3211089
Clean up array access.
mportiz08 May 5, 2023
596a344
Merge branch 'main' into render-all-attributes-on-one-line
mportiz08 May 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 9 additions & 12 deletions src/components/DocumentationTopic/PrimaryContent/DeclarationSource.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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`;
}

Expand All @@ -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
Expand Down
139 changes: 62 additions & 77 deletions tests/unit/components/DocumentationTopic/PrimaryContent/DeclarationSource.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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', () => {
Expand All @@ -185,7 +183,7 @@ describe('Swift function/initializer formatting', () => {
// After:
// func foo(
// _ a: A,
// _ b: B,
// _ b: B
// ) -> Bar
const tokens = [
{
Expand Down Expand Up @@ -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', () => {
Expand All @@ -288,7 +276,7 @@ describe('Swift function/initializer formatting', () => {
// After:
// func foo(
// _ a: A,
// _ b: B,
// _ b: B
// ) -> (A, B)
const tokens = [
{
Expand Down Expand Up @@ -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 = [
{
Expand Down Expand Up @@ -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', () => {
Expand All @@ -600,7 +568,7 @@ describe('Swift function/initializer formatting', () => {
// After:
// func foo(
// _ a: A,
// _ b: B,
// _ b: B
// ) -> Bar
const tokens = [
{
Expand Down Expand Up @@ -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 = [
{
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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`,
);
});
});