Skip to content

Commit 8d8a25f

Browse files
hanna-skrylhanna-skryl
committed
feat(utils): add type guards and URL source formatting
1 parent 81552c1 commit 8d8a25f

9 files changed

Lines changed: 131 additions & 39 deletions

File tree

‎packages/utils/src/index.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,11 @@ export {
125125
listGroupsFromAllPlugins,
126126
} from './lib/reports/flatten-plugins.js';
127127
export { formatIssueSeverities, wrapTags } from './lib/reports/formatting.js';
128+
export {
129+
isFileIssue,
130+
isFileSource,
131+
isUrlSource,
132+
} from './lib/reports/type-guards.js';
128133
export { generateMdReport } from './lib/reports/generate-md-report.js';
129134
export {
130135
generateMdReportsDiff,

‎packages/utils/src/lib/reports/__snapshots__/generate-md-report.unit.test.ts.snap‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ exports[`auditDetails > should render complete details section 1`] = `
2626
2727
#### Issues
2828
29-
| Severity | Message | Source file | Line(s) |
29+
| Severity | Message | Source | Line(s) |
3030
| :--------: | :---------------------------------------------- | :------------------ | :-----: |
3131
| 🚨 _error_ | Use design system components instead of classes | \`list.component.ts\` | 400-200 |
3232
| 🚨 _error_ | File size is 20KB too big | \`list.component.ts\` | |
@@ -38,7 +38,7 @@ exports[`auditDetails > should render complete details section 1`] = `
3838
exports[`auditDetailsIssues > should render complete section 1`] = `
3939
"#### Issues
4040
41-
| Severity | Message | Source file | Line(s) |
41+
| Severity | Message | Source | Line(s) |
4242
| :----------: | :--------------------------------- | :------------- | :-----: |
4343
| ℹ️ _info_ | File \`index.js\` is 56Kb. | \`index.js\` | |
4444
| ⚠️ _warning_ | Package license is has to be "MIT" | \`package.json\` | 4 |
@@ -130,7 +130,7 @@ Search engines are unable to include your pages in search results if they don't
130130
131131
#### Issues
132132
133-
| Severity | Message | Source file | Line(s) |
133+
| Severity | Message | Source | Line(s) |
134134
| :----------: | :--------------------------------- | :------------- | :-----: |
135135
| ℹ️ _info_ | File \`index.js\` is 56Kb. | \`index.js\` | |
136136
| ⚠️ _warning_ | Package license is has to be "MIT" | \`package.json\` | 4 |

‎packages/utils/src/lib/reports/__snapshots__/report.md‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ Performance metrics [📖 Docs](https://developers.google.com/web/fundamentals/p
7373

7474
#### Issues
7575

76-
| Severity | Message | Source file | Line(s) |
76+
| Severity | Message | Source | Line(s) |
7777
| :----------: | :----------------------------------------------- | :------------------------------ | :-----: |
7878
| ⚠️ _warning_ | 'onCreate' is missing in props validation | `src/components/CreateTodo.jsx` | 15 |
7979
| ⚠️ _warning_ | 'setQuery' is missing in props validation | `src/components/TodoFilter.jsx` | 10 |
@@ -93,7 +93,7 @@ ESLint rule **prop-types**, from _react_ plugin. [📖 Docs](https://github.com/
9393

9494
#### Issues
9595

96-
| Severity | Message | Source file | Line(s) |
96+
| Severity | Message | Source | Line(s) |
9797
| :----------: | :----------------------------------------------------------------- | :---------------------- | :-----: |
9898
| ⚠️ _warning_ | 'data' is already declared in the upper scope on line 5 column 10. | `src/hooks/useTodos.js` | 11 |
9999
| ⚠️ _warning_ | 'data' is already declared in the upper scope on line 5 column 10. | `src/hooks/useTodos.js` | 29 |
@@ -110,7 +110,7 @@ ESLint rule **no-shadow**. [📖 Docs](https://eslint.org/docs/latest/rules/no-s
110110

111111
#### Issues
112112

113-
| Severity | Message | Source file | Line(s) |
113+
| Severity | Message | Source | Line(s) |
114114
| :----------: | :--------------------------- | :---------------------- | :-----: |
115115
| ⚠️ _warning_ | Expected property shorthand. | `src/hooks/useTodos.js` | 19 |
116116
| ⚠️ _warning_ | Expected property shorthand. | `src/hooks/useTodos.js` | 32 |
@@ -127,7 +127,7 @@ ESLint rule **object-shorthand**. [📖 Docs](https://eslint.org/docs/latest/rul
127127

128128
#### Issues
129129

130-
| Severity | Message | Source file | Line(s) |
130+
| Severity | Message | Source | Line(s) |
131131
| :----------: | :----------------------------------------------------------------------------------------------------------------------- | :---------------------- | :-----: |
132132
| ⚠️ _warning_ | React Hook useCallback does nothing when called with only one argument. Did you forget to pass an array of dependencies? | `src/hooks/useTodos.js` | 17 |
133133
| ⚠️ _warning_ | React Hook useCallback does nothing when called with only one argument. Did you forget to pass an array of dependencies? | `src/hooks/useTodos.js` | 40 |
@@ -143,7 +143,7 @@ ESLint rule **exhaustive-deps**, from _react-hooks_ plugin. [📖 Docs](https://
143143

144144
#### Issues
145145

146-
| Severity | Message | Source file | Line(s) |
146+
| Severity | Message | Source | Line(s) |
147147
| :----------: | :----------------------------------------- | :---------------------------- | :-----: |
148148
| ⚠️ _warning_ | Missing "key" prop for element in iterator | `src/components/TodoList.jsx` | 7-28 |
149149

@@ -158,7 +158,7 @@ ESLint rule **jsx-key**, from _react_ plugin. [📖 Docs](https://github.com/jsx
158158

159159
#### Issues
160160

161-
| Severity | Message | Source file | Line(s) |
161+
| Severity | Message | Source | Line(s) |
162162
| :----------: | :-------------------------------------------- | :------------ | :-----: |
163163
| ⚠️ _warning_ | 'loading' is assigned a value but never used. | `src/App.jsx` | 8 |
164164

@@ -173,7 +173,7 @@ ESLint rule **no-unused-vars**. [📖 Docs](https://eslint.org/docs/latest/rules
173173

174174
#### Issues
175175

176-
| Severity | Message | Source file | Line(s) |
176+
| Severity | Message | Source | Line(s) |
177177
| :----------: | :------------------------------------------------------------- | :---------------------- | :-----: |
178178
| ⚠️ _warning_ | Arrow function has too many lines (71). Maximum allowed is 50. | `src/hooks/useTodos.js` | 3-73 |
179179

@@ -188,7 +188,7 @@ ESLint rule **max-lines-per-function**. [📖 Docs](https://eslint.org/docs/late
188188

189189
#### Issues
190190

191-
| Severity | Message | Source file | Line(s) |
191+
| Severity | Message | Source | Line(s) |
192192
| :----------: | :----------------------------------------------- | :-------------- | :-----: |
193193
| ⚠️ _warning_ | 'root' is never reassigned. Use 'const' instead. | `src/index.jsx` | 5 |
194194

@@ -203,7 +203,7 @@ ESLint rule **prefer-const**. [📖 Docs](https://eslint.org/docs/latest/rules/p
203203

204204
#### Issues
205205

206-
| Severity | Message | Source file | Line(s) |
206+
| Severity | Message | Source | Line(s) |
207207
| :----------: | :----------------------------------------------------------------------------------------------------- | :------------------------------ | :-----: |
208208
| ⚠️ _warning_ | Unexpected block statement surrounding arrow body; move the returned value immediately after the `=>`. | `src/components/TodoFilter.jsx` | 3-25 |
209209

@@ -218,7 +218,7 @@ ESLint rule **arrow-body-style**. [📖 Docs](https://eslint.org/docs/latest/rul
218218

219219
#### Issues
220220

221-
| Severity | Message | Source file | Line(s) |
221+
| Severity | Message | Source | Line(s) |
222222
| :----------: | :----------------------------------- | :---------------------- | :-----: |
223223
| ⚠️ _warning_ | Expected '===' and instead saw '=='. | `src/hooks/useTodos.js` | 41 |
224224

‎packages/utils/src/lib/reports/formatting.ts‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@ import type {
99
AuditReport,
1010
Issue,
1111
IssueSeverity,
12+
IssueSource,
1213
SourceFileLocation,
14+
SourceUrlLocation,
1315
Table,
1416
Tree,
1517
} from '@code-pushup/models';
@@ -27,6 +29,7 @@ import {
2729
getGitHubBaseUrl,
2830
getGitLabBaseUrl,
2931
} from './environment-type.js';
32+
import { isUrlSource } from './type-guards.js';
3033
import type { MdReportOptions } from './types.js';
3134
import { compareIssueSeverity } from './utils.js';
3235

@@ -113,6 +116,32 @@ export function linkToLocalSourceForIde(
113116
return md.link(formatFileLink(file, position, outputDir), md.code(file));
114117
}
115118

119+
/**
120+
* Link to URL source
121+
* @param source URL-based source location
122+
*
123+
* @example
124+
* linkToUrlSource({ url: 'https://example.com', selector: 'img.logo' }) // [`img.logo`](https://example.com)
125+
*/
126+
export function linkToUrlSource(source: SourceUrlLocation): InlineText {
127+
const { url, selector } = source;
128+
const displayText = selector ? md.code(selector) : url;
129+
return md.link(url, displayText);
130+
}
131+
132+
/**
133+
* Link to source (handles both file and URL sources)
134+
*/
135+
export function linkToSource(
136+
source: IssueSource,
137+
options?: Pick<MdReportOptions, 'outputDir'>,
138+
): InlineText {
139+
if (isUrlSource(source)) {
140+
return linkToUrlSource(source);
141+
}
142+
return linkToLocalSourceForIde(source, options);
143+
}
144+
116145
export function formatSourceLine(
117146
position: SourceFileLocation['position'],
118147
): string {

‎packages/utils/src/lib/reports/generate-md-report.ts‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {
1010
} from './constants.js';
1111
import {
1212
formatSourceLine,
13-
linkToLocalSourceForIde,
13+
linkToSource,
1414
metaDescription,
1515
tableSection,
1616
treeSection,
@@ -20,6 +20,7 @@ import {
2020
categoriesDetailsSection,
2121
categoriesOverviewSection,
2222
} from './generate-md-report-category-section.js';
23+
import { isFileSource } from './type-guards.js';
2324
import type { MdReportOptions, ScoredReport } from './types.js';
2425
import {
2526
formatReportScore,
@@ -82,7 +83,7 @@ export function auditDetailsIssues(
8283
[
8384
{ heading: 'Severity', alignment: 'center' },
8485
{ heading: 'Message', alignment: 'left' },
85-
{ heading: 'Source file', alignment: 'left' },
86+
{ heading: 'Source', alignment: 'left' },
8687
{ heading: 'Line(s)', alignment: 'center' },
8788
],
8889
issues.map(({ severity: level, message, source }: Issue) => {
@@ -92,12 +93,15 @@ export function auditDetailsIssues(
9293
if (!source) {
9394
return [severity, formattedMessage];
9495
}
95-
const file = linkToLocalSourceForIde(source, options);
96-
if (!source.position) {
97-
return [severity, formattedMessage, file];
96+
97+
const sourceLink = linkToSource(source, options);
98+
99+
if (isFileSource(source) && source.position) {
100+
const line = formatSourceLine(source.position);
101+
return [severity, formattedMessage, sourceLink, line];
98102
}
99-
const line = formatSourceLine(source.position);
100-
return [severity, formattedMessage, file, line];
103+
104+
return [severity, formattedMessage, sourceLink];
101105
}),
102106
);
103107
}

‎packages/utils/src/lib/reports/generate-md-report.unit.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -492,7 +492,7 @@ describe('auditsSection', () => {
492492
expect(md).toContainMarkdownTableRow([
493493
'Severity',
494494
'Message',
495-
'Source file',
495+
'Source',
496496
'Line(s)',
497497
]);
498498
expect(md).toContainMarkdownTableRow(['value']);

‎packages/utils/src/lib/reports/type-guards.ts‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import type {
2+
FileIssue,
3+
Issue,
4+
IssueSource,
5+
SourceFileLocation,
6+
SourceUrlLocation,
7+
UrlIssue,
8+
} from '@code-pushup/models';
9+
10+
/** Type guard for file-based source */
11+
export function isFileSource(
12+
source: IssueSource,
13+
): source is SourceFileLocation {
14+
return 'file' in source;
15+
}
16+
17+
/** Type guard for URL-based source */
18+
export function isUrlSource(source: IssueSource): source is SourceUrlLocation {
19+
return 'url' in source;
20+
}
21+
22+
/** Type guard for issue with file source */
23+
export function isFileIssue(issue: Issue): issue is FileIssue {
24+
return issue.source != null && isFileSource(issue.source);
25+
}
26+
27+
/** Type guard for issue with URL source */
28+
export function isUrlIssue(issue: Issue): issue is UrlIssue {
29+
return issue.source != null && isUrlSource(issue.source);
30+
}

‎packages/utils/src/lib/reports/utils.ts‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,11 @@ import type {
77
IssueSeverity as CliIssueSeverity,
88
Group,
99
Issue,
10+
IssueSource,
11+
SourceFileLocation,
1012
} from '@code-pushup/models';
1113
import { SCORE_COLOR_RANGE } from './constants.js';
14+
import { isFileSource } from './type-guards.js';
1215
import type {
1316
ScoreFilter,
1417
ScoredReport,
@@ -241,6 +244,10 @@ export function getPluginNameFromSlug(
241244
);
242245
}
243246

247+
function getSourceIdentifier(source: IssueSource): string {
248+
return isFileSource(source) ? source.file : source.url;
249+
}
250+
244251
export function compareIssues(a: Issue, b: Issue): number {
245252
if (a.severity !== b.severity) {
246253
return -compareIssueSeverity(a.severity, b.severity);
@@ -251,15 +258,22 @@ export function compareIssues(a: Issue, b: Issue): number {
251258
if (a.source && !b.source) {
252259
return 1;
253260
}
254-
if (a.source?.file !== b.source?.file) {
255-
return a.source?.file.localeCompare(b.source?.file || '') ?? 0;
261+
if (a.source && b.source) {
262+
const aId = getSourceIdentifier(a.source);
263+
const bId = getSourceIdentifier(b.source);
264+
if (aId !== bId) {
265+
return aId.localeCompare(bId);
266+
}
267+
if (isFileSource(a.source) && isFileSource(b.source)) {
268+
return compareSourceFilePosition(a.source.position, b.source.position);
269+
}
256270
}
257-
return compareSourceFilePosition(a.source?.position, b.source?.position);
271+
return 0;
258272
}
259273

260274
function compareSourceFilePosition(
261-
a: NonNullable<Issue['source']>['position'],
262-
b: NonNullable<Issue['source']>['position'],
275+
a: SourceFileLocation['position'],
276+
b: SourceFileLocation['position'],
263277
): number {
264278
if (!a && b) {
265279
return -1;

‎testing/test-fixtures/src/lib/utils/os-agnostic.ts‎

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,15 @@
1-
import type { AuditOutput, AuditReport } from '@code-pushup/models';
1+
import type {
2+
AuditOutput,
3+
AuditReport,
4+
IssueSource,
5+
SourceFileLocation,
6+
} from '@code-pushup/models';
27
import { osAgnosticPath } from '@code-pushup/test-utils';
38

9+
function isFileSource(source: IssueSource): source is SourceFileLocation {
10+
return 'file' in source;
11+
}
12+
413
export function osAgnosticAudit<T extends AuditOutput | AuditReport>(
514
audit: T,
615
transformMessage: (message: string) => string = s => s,
@@ -12,18 +21,19 @@ export function osAgnosticAudit<T extends AuditOutput | AuditReport>(
1221
return {
1322
...audit,
1423
details: {
15-
issues: issues.map(issue =>
16-
issue.source == null
17-
? issue
18-
: {
19-
...issue,
20-
source: {
21-
...issue.source,
22-
file: osAgnosticPath(issue.source.file),
23-
},
24-
message: transformMessage(issue.message),
25-
},
26-
),
24+
issues: issues.map(issue => {
25+
if (issue.source == null || !isFileSource(issue.source)) {
26+
return issue;
27+
}
28+
return {
29+
...issue,
30+
source: {
31+
...issue.source,
32+
file: osAgnosticPath(issue.source.file),
33+
},
34+
message: transformMessage(issue.message),
35+
};
36+
}),
2737
},
2838
};
2939
}

0 commit comments

Comments
 (0)