Skip to content

Commit

Permalink
feat: fix function params order
Browse files Browse the repository at this point in the history
issue: #60
  • Loading branch information
hosseinmd committed Feb 7, 2021
1 parent 54bdbc5 commit 2524413
Show file tree
Hide file tree
Showing 4 changed files with 207 additions and 54 deletions.
162 changes: 109 additions & 53 deletions src/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
formatType,
detectEndOfLine,
} from "./utils";
import { DESCRIPTION } from "./tags";
import { DESCRIPTION, PARAM } from "./tags";
import {
TAGS_DESCRIPTION_NEEDED,
TAGS_GROUP,
Expand Down Expand Up @@ -54,6 +54,7 @@ export const getParser = (parser: Parser["parse"]) =>

ast.comments.forEach((comment) => {
if (!isBlockComment(comment)) return;
const tokenIndex = ast.tokens.findIndex(({ loc }) => loc === comment.loc);

This comment has been minimized.

Copy link
@RunDevelopment

RunDevelopment Feb 9, 2021

Contributor

This is something I wanted to ask @hosseinmd: Could this be a potential performance problem?

If we highlighted a large document with a large number of comments, then going through all tokens linearly might be a bottleneck. One could construct a text for which this plugin will now take O(n^2) time to format leaving the door open for a potential algorithmic complexity attack. Do people highlight unknown/untrusted code with Prettier? Could that be a problem or am I just being paranoid...

This comment has been minimized.

Copy link
@hosseinmd

hosseinmd Feb 9, 2021

Author Owner

Yes, there is a lot of code which has performance problem. Give me a solution for them (Please don't refactor, rename or anything's which not needed).

This comment has been minimized.

Copy link
@RunDevelopment

RunDevelopment Feb 9, 2021

Contributor

Sure!

Please don't refactor, rename or anything's which not needed

Don't worry, that will be its own PR ;)


/** Issue: https://github.com/hosseinmd/prettier-plugin-jsdoc/issues/18 */
comment.value = comment.value.replace(/^([*]+)/g, "*");
Expand Down Expand Up @@ -95,37 +96,119 @@ export const getParser = (parser: Parser["parse"]) =>

parsed.tags
// Prepare tags data
.map(({ type, optional, ...rest }) => {
if (type) {
/**
* Convert optional to standard
* https://jsdoc.app/tags-type.html#:~:text=Optional%20parameter
*/
type = type.replace(/[=]$/, () => {
optional = true;
return "";
});

type = convertToModernType(type);
type = formatType(type, {
...options,
printWidth: commentContentPrintWidth,
});
}

return {
...rest,
type,
optional,
} as commentParser.Tag;
})

// Group tags
.reduce<commentParser.Tag[][]>((tagGroups, cur, index, array) => {
if (
(tagGroups.length === 0 || TAGS_GROUP.includes(cur.tag)) &&
array[index - 1]?.tag !== DESCRIPTION
) {
tagGroups.push([]);
}
tagGroups[tagGroups.length - 1].push(cur);

return tagGroups;
}, [])
.flatMap((tagGroup, index, tags) => {
let paramsOrder: string[] | undefined;
// next token
const nextTokenType = ast.tokens[tokenIndex + 1]?.type;
if (
typeof nextTokenType === "object" &&
nextTokenType.label === "function"
) {
try {
let openedParenthesesCount = 1;
const endIndex = ast.tokens
.slice(tokenIndex + 4)
.findIndex(({ type }) => {
if (typeof type === "string") {
return false;
} else if (type.label === "(") {
openedParenthesesCount++;
} else if (type.label === ")") {
openedParenthesesCount--;
}

return openedParenthesesCount === 0;
});

const params = ast.tokens.slice(
tokenIndex + 4,
tokenIndex + 4 + endIndex + 1,
);

paramsOrder = params
.filter(
({ type }) =>
typeof type === "object" && type.label === "name",
)
.map(({ value }) => value);
} catch {
//
}
}

// sort tags within groups
tagGroup.sort((a, b) => {
if (
paramsOrder &&
paramsOrder.length > 1 &&
a.tag === PARAM &&
b.tag === PARAM
) {
//sort params
return paramsOrder.indexOf(a.name) - paramsOrder.indexOf(b.name);
}
return getTagOrderWeight(a.tag) - getTagOrderWeight(b.tag);
});

// add an empty line between groups
if (tags.length - 1 !== index) {
tagGroup.push(SPACE_TAG_DATA);
}

return tagGroup;
})
.map(
({
name,
description,
type,
tag,
source,
name,
optional,
default: _default,
...restInfo
description,
tag,
...rest
}) => {
const isVerticallyAlignAbleTags = TAGS_VERTICALLY_ALIGN_ABLE.includes(
tag,
);

if (type) {
/**
* Convert optional to standard
* https://jsdoc.app/tags-type.html#:~:text=Optional%20parameter
*/
type = type.replace(/[=]$/, () => {
optional = true;
return "";
});

type = convertToModernType(type);
type = formatType(type, {
...options,
printWidth: commentContentPrintWidth,
});

// Additional operations on name
if (name) {
// Optional tag name
Expand Down Expand Up @@ -157,43 +240,16 @@ export const getParser = (parser: Parser["parse"]) =>
description = description.trim();

return {
...restInfo,
type,
name,
optional,
default: _default,
description,
type,
tag,
source,
default: _default,
optional,
} as commentParser.Tag;
...rest,
};
},
)

// Group tags
.reduce<commentParser.Tag[][]>((tagGroups, cur, index, array) => {
if (
(tagGroups.length === 0 || TAGS_GROUP.includes(cur.tag)) &&
array[index - 1]?.tag !== DESCRIPTION
) {
tagGroups.push([]);
}
tagGroups[tagGroups.length - 1].push(cur);

return tagGroups;
}, [])
.flatMap((tagGroup, index, tags) => {
// sort tags within groups
tagGroup.sort((a, b) => {
return getTagOrderWeight(a.tag) - getTagOrderWeight(b.tag);
});

// add an empty line between groups
if (tags.length - 1 !== index) {
tagGroup.push(SPACE_TAG_DATA);
}

return tagGroup;
})
.filter(({ description, tag }) => {
if (!description && TAGS_DESCRIPTION_NEEDED.includes(tag)) {
return false;
Expand Down
23 changes: 22 additions & 1 deletion src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ type LocationDetails = { line: number; column: number };
type Location = { start: LocationDetails; end: LocationDetails };

export type PrettierComment = {
type: "CommentBlock";
type: "CommentBlock" | "Block";
value: string;
start: number;
end: number;
Expand All @@ -36,4 +36,25 @@ export type AST = {
directives: [];
};
comments: PrettierComment[];
tokens: {
type:
| "CommentBlock"
| "Block"
| {
label: string; // "function" | "name";
keyword?: string;
beforeExpr: boolean;
startsExpr: boolean;
rightAssociative: boolean;
isLoop: boolean;
isAssign: boolean;
prefix: boolean;
postfix: boolean;
binop: null;
};
value: string;
start: number;
end: number;
loc: Location;
}[];
};
35 changes: 35 additions & 0 deletions tests/__snapshots__/main.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,41 @@ exports[`Sould keep params ordering when more than 10 tags are present 1`] = `
"
`;

exports[`param order 1`] = `
"/**
* @param {string} param0 Description
* @param {object} param1 Description
* @param {number} param2 Description
*/
function fun(param0, param1, param2) {}
export const SubDomain = {
/**
* @param subDomainAddress2
* @param {any} subDomainAddress
* @returns {import(\\"axios\\").AxiosResponse<import(\\"../types\\").SubDomain>}
*/
async subDomain(subDomainAddress2, subDomainAddress) {},
};
/**
* @param {string} param0 Description
* @param {object} param1 Description
* @param {number} param2 Description
*/
function fun(param0: string, param1: {}, param2: () => {}) {}
/**
* @param {string} param0 Description
* @param {number} param2 Description
* @param {object} param1 Description
*/
const fun = (param0, param1, param2) => {
console.log(\\"\\");
};
"
`;

exports[`since 1`] = `
"/** @since 3.16.0 */
"
Expand Down
41 changes: 41 additions & 0 deletions tests/main.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -475,3 +475,44 @@ function a() {}
expect(subject(text_lf, { endOfLine: "auto" })).toEqual(formatted_lf);
expect(subject(text_crlf, { endOfLine: "auto" })).toEqual(formatted_crlf);
});

test("param order", () => {
const result = subject(`
/**
* @param { string } param0 description
* @param { number } param2 description
* @param { object } param1 description
*/
function fun(param0, param1, param2){}
export const SubDomain = {
/**
* @param {} subDomainAddress2
* @param {any} subDomainAddress
* @returns {import('axios').AxiosResponse<import('../types').SubDomain>}
*/
async subDomain(subDomainAddress2,subDomainAddress) {
},
};
/**
* @param { string } param0 description
* @param { number } param2 description
* @param { object } param1 description
*/
function fun(param0:string, param1:{}, param2:()=>{}){}
/**
* @param { string } param0 description
* @param { number } param2 description
* @param { object } param1 description
*/
const fun=(param0, param1, param2)=>{
console.log('')
}
`);

expect(result).toMatchSnapshot();
});

0 comments on commit 2524413

Please sign in to comment.