Skip to content

Commit 7960240

Browse files
Copilotalexr00
andauthored
Add ESLint rule to enforce "pull request" over "PR" in user-facing strings (#7693)
* Initial plan * Initial plan for PR lint rule implementation Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com> * Implement ESLint rule to detect PR in user-facing strings - Add no-pr-in-user-strings.js rule to detect PR in vscode.l10n.t() calls - Export rule from build/eslint-rules/index.js - Configure rule as error level in .eslintrc.js - Rule uses word boundary regex to detect standalone 'PR' - Successfully detects existing issue in issueFeatureRegistrar.ts The rule correctly identifies 1 existing violation which will need to be fixed in a follow-up commit to demonstrate the rule is working properly. * Fix PR reference in user-facing string to use pull request Replace 'PR' with 'pull request' in @githubpr Summarize command to comply with new ESLint rule Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com> * remove proposed change --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com>
1 parent 90e6a83 commit 7960240

4 files changed

Lines changed: 100 additions & 2 deletions

File tree

.eslintrc.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@ module.exports = {
2828
{
2929
files: ['**/*.ts', '**/*.tsx'],
3030
rules: {
31-
'rulesdir/no-any-except-union-method-signature': 'error'
31+
'rulesdir/no-any-except-union-method-signature': 'error',
32+
'rulesdir/no-pr-in-user-strings': 'error'
3233
}
3334
}
3435
]

build/eslint-rules/index.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,5 @@
88
module.exports = {
99
'public-methods-well-defined-types': require('./public-methods-well-defined-types'),
1010
'no-any-except-union-method-signature': require('./no-any-except-union-method-signature'),
11+
'no-pr-in-user-strings': require('./no-pr-in-user-strings'),
1112
};
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
'use strict';
7+
8+
/**
9+
* ESLint rule to detect the string "PR" in user-facing strings and suggest using "pull request" instead.
10+
* This rule checks:
11+
* - String literals passed to vscode.l10n.t() calls
12+
* - String literals passed to l10n.t() calls
13+
*/
14+
15+
module.exports = {
16+
meta: {
17+
type: 'problem',
18+
docs: {
19+
description: 'Detect "PR" in user-facing strings and suggest using "pull request" instead',
20+
category: 'Best Practices',
21+
recommended: true,
22+
},
23+
schema: [],
24+
messages: {
25+
noPrInUserString: 'Use "pull request" instead of "PR" in user-facing strings. Found: {{foundText}}',
26+
},
27+
},
28+
29+
create(context) {
30+
/**
31+
* Check if a string contains "PR" as a standalone word
32+
*/
33+
function containsPR(str) {
34+
// Use word boundary regex to match "PR" as a standalone word
35+
const prRegex = /\bPR\b/;
36+
return prRegex.test(str);
37+
}
38+
39+
/**
40+
* Check if a node is a call to vscode.l10n.t or l10n.t
41+
*/
42+
function isL10nTCall(node) {
43+
if (node.type !== 'CallExpression') {
44+
return false;
45+
}
46+
47+
const callee = node.callee;
48+
49+
// Handle l10n.t() calls
50+
if (callee.type === 'MemberExpression' &&
51+
callee.property &&
52+
callee.property.name === 't') {
53+
54+
// Check for vscode.l10n.t
55+
if (callee.object.type === 'MemberExpression' &&
56+
callee.object.object &&
57+
callee.object.object.name === 'vscode' &&
58+
callee.object.property &&
59+
callee.object.property.name === 'l10n') {
60+
return true;
61+
}
62+
63+
// Check for l10n.t
64+
if (callee.object.type === 'Identifier' &&
65+
callee.object.name === 'l10n') {
66+
return true;
67+
}
68+
}
69+
70+
return false;
71+
}
72+
73+
return {
74+
// Check CallExpression nodes for l10n.t calls
75+
CallExpression(node) {
76+
if (isL10nTCall(node)) {
77+
// Check the first argument (string literal)
78+
if (node.arguments && node.arguments.length > 0) {
79+
const firstArg = node.arguments[0];
80+
if (firstArg.type === 'Literal' && typeof firstArg.value === 'string') {
81+
if (containsPR(firstArg.value)) {
82+
context.report({
83+
node: firstArg,
84+
messageId: 'noPrInUserString',
85+
data: {
86+
foundText: firstArg.value
87+
}
88+
});
89+
}
90+
}
91+
}
92+
}
93+
}
94+
};
95+
}
96+
};

src/issues/issueFeatureRegistrar.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -510,7 +510,7 @@ export class IssueFeatureRegistrar extends Disposable {
510510
} else {
511511
const pullRequestModel = issue.pullRequestModel;
512512
const remote = pullRequestModel.githubRepository.remote;
513-
commands.executeCommand(chatCommandID, vscode.l10n.t('@githubpr Summarize PR {0}/{1}#{2}', remote.owner, remote.repositoryName, pullRequestModel.number));
513+
commands.executeCommand(chatCommandID, vscode.l10n.t('@githubpr Summarize pull request {0}/{1}#{2}', remote.owner, remote.repositoryName, pullRequestModel.number));
514514
}
515515
}),
516516
);

0 commit comments

Comments
 (0)