Skip to content

Commit ce9bfaf

Browse files
vaindclaude
andcommitted
Simplify checkLegalBoilerplate and remove unnecessary indirection
Extract findPRTemplate helper and named constants (INTERNAL_ASSOCIATIONS, PR_TEMPLATE_PATHS). Simplify extractLegalBoilerplateSection using findIndex. Inline checkLegalBoilerplate call in checkAll, removing the wrapper function. Derive test LEGAL_TEXT from the template constant to prevent drift. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 072b08a commit ce9bfaf

File tree

3 files changed

+51
-86
lines changed

3 files changed

+51
-86
lines changed

danger/dangerfile-utils.js

Lines changed: 46 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -86,100 +86,64 @@ function extractPRFlavor(prTitle, prBranchRef) {
8686
return "";
8787
}
8888

89-
/**
90-
* Extract the legal boilerplate section from the PR template
91-
* @param {string} templateContent - The PR template content
92-
* @returns {string} The extracted legal boilerplate section
93-
*/
94-
function extractLegalBoilerplateSection(templateContent) {
95-
// Find the legal boilerplate section and extract it
96-
const lines = templateContent.split('\n');
97-
let inLegalSection = false;
98-
let legalSection = [];
99-
100-
for (let i = 0; i < lines.length; i++) {
101-
const line = lines[i];
102-
103-
// Check if this line is the legal boilerplate header
104-
if (/^#{1,6}\s+Legal\s+Boilerplate/i.test(line)) {
105-
inLegalSection = true;
106-
legalSection.push(line);
107-
continue;
108-
}
109-
110-
// If we're in the legal section
111-
if (inLegalSection) {
112-
// Check if we've reached another header (end of legal section)
113-
if (/^#{1,6}\s+/.test(line)) {
114-
break;
115-
}
116-
legalSection.push(line);
89+
/** @returns {string} The legal boilerplate section extracted from the content, or empty string if none found */
90+
function extractLegalBoilerplateSection(content) {
91+
const lines = content.split('\n');
92+
const legalHeaderIndex = lines.findIndex(line => /^#{1,6}\s+Legal\s+Boilerplate/i.test(line));
93+
94+
if (legalHeaderIndex === -1) {
95+
return '';
96+
}
97+
98+
const sectionLines = [lines[legalHeaderIndex]];
99+
100+
for (let i = legalHeaderIndex + 1; i < lines.length; i++) {
101+
if (/^#{1,6}\s+/.test(lines[i])) {
102+
break;
117103
}
104+
sectionLines.push(lines[i]);
118105
}
119-
120-
return legalSection.join('\n').trim();
106+
107+
return sectionLines.join('\n').trim();
121108
}
122109

110+
const INTERNAL_ASSOCIATIONS = ['OWNER', 'MEMBER', 'COLLABORATOR'];
111+
112+
const PR_TEMPLATE_PATHS = [
113+
'.github/PULL_REQUEST_TEMPLATE.md',
114+
'.github/pull_request_template.md',
115+
'PULL_REQUEST_TEMPLATE.md',
116+
'pull_request_template.md',
117+
'.github/PULL_REQUEST_TEMPLATE/pull_request_template.md'
118+
];
119+
123120
/**
124121
* Check that external contributors include the required legal boilerplate in their PR body.
125122
* Accepts danger context and reporting functions as parameters for testability.
126-
*
127-
* @param {object} options
128-
* @param {object} options.danger - The DangerJS danger object
129-
* @param {Function} options.fail - DangerJS fail function
130-
* @param {Function} options.markdown - DangerJS markdown function
131123
*/
132124
async function checkLegalBoilerplate({ danger, fail, markdown }) {
133125
console.log('::debug:: Checking legal boilerplate requirements...');
134126

135-
// Check if the PR author is an external contributor using author_association
136127
const authorAssociation = danger.github.pr.author_association;
137-
const isExternalContributor = !['OWNER', 'MEMBER', 'COLLABORATOR'].includes(authorAssociation);
138-
139-
if (!isExternalContributor) {
128+
if (INTERNAL_ASSOCIATIONS.includes(authorAssociation)) {
140129
console.log('::debug:: Skipping legal boilerplate check for organization member/collaborator');
141130
return;
142131
}
143132

144-
// Find PR template
145-
let prTemplateContent = null;
146-
const possibleTemplatePaths = [
147-
'.github/PULL_REQUEST_TEMPLATE.md',
148-
'.github/pull_request_template.md',
149-
'PULL_REQUEST_TEMPLATE.md',
150-
'pull_request_template.md',
151-
'.github/PULL_REQUEST_TEMPLATE/pull_request_template.md'
152-
];
153-
154-
for (const templatePath of possibleTemplatePaths) {
155-
const content = await danger.github.utils.fileContents(templatePath);
156-
if (content) {
157-
prTemplateContent = content;
158-
console.log(`::debug:: Found PR template at ${templatePath}`);
159-
break;
160-
}
161-
}
162-
133+
const prTemplateContent = await findPRTemplate(danger);
163134
if (!prTemplateContent) {
164135
console.log('::debug:: No PR template found, skipping legal boilerplate check');
165136
return;
166137
}
167138

168-
// Check if template contains a Legal Boilerplate section
169-
const legalBoilerplateHeaderRegex = /^#{1,6}\s+Legal\s+Boilerplate/im;
170-
if (!legalBoilerplateHeaderRegex.test(prTemplateContent)) {
139+
const expectedBoilerplate = extractLegalBoilerplateSection(prTemplateContent);
140+
if (!expectedBoilerplate) {
171141
console.log('::debug:: PR template does not contain a Legal Boilerplate section');
172142
return;
173143
}
174144

175-
// Extract expected boilerplate from template
176-
const expectedBoilerplate = extractLegalBoilerplateSection(prTemplateContent);
177-
const prBody = danger.github.pr.body || '';
178-
179-
// Extract actual boilerplate from PR body
180-
const actualBoilerplate = extractLegalBoilerplateSection(prBody);
145+
const actualBoilerplate = extractLegalBoilerplateSection(danger.github.pr.body || '');
181146

182-
// Check if PR body contains the legal boilerplate section
183147
if (!actualBoilerplate) {
184148
fail('This PR is missing the required legal boilerplate. As an external contributor, please include the "Legal Boilerplate" section from the PR template in your PR description.');
185149

@@ -199,13 +163,10 @@ This is required to ensure proper intellectual property rights for your contribu
199163
return;
200164
}
201165

202-
// Verify the actual boilerplate matches the expected one
203-
// Normalize whitespace for comparison
166+
// Normalize whitespace so minor formatting differences don't cause false negatives
204167
const normalizeWhitespace = (str) => str.replace(/\s+/g, ' ').trim();
205-
const expectedNormalized = normalizeWhitespace(expectedBoilerplate);
206-
const actualNormalized = normalizeWhitespace(actualBoilerplate);
207168

208-
if (expectedNormalized !== actualNormalized) {
169+
if (normalizeWhitespace(expectedBoilerplate) !== normalizeWhitespace(actualBoilerplate)) {
209170
fail('The legal boilerplate in your PR description does not match the template. Please ensure you include the complete, unmodified legal text from the PR template.');
210171

211172
markdown(`
@@ -227,6 +188,18 @@ This is required to ensure proper intellectual property rights for your contribu
227188
console.log('::debug:: Legal boilerplate validated successfully ✓');
228189
}
229190

191+
/** Try each known PR template path and return the first one with content. */
192+
async function findPRTemplate(danger) {
193+
for (const templatePath of PR_TEMPLATE_PATHS) {
194+
const content = await danger.github.utils.fileContents(templatePath);
195+
if (content) {
196+
console.log(`::debug:: Found PR template at ${templatePath}`);
197+
return content;
198+
}
199+
}
200+
return null;
201+
}
202+
230203
module.exports = {
231204
FLAVOR_CONFIG,
232205
getFlavorConfig,

danger/dangerfile-utils.test.js

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -536,13 +536,10 @@ Look, I get it. The entity doing business as "Sentry" was incorporated in the St
536536
## Checklist
537537
- [ ] Tests added`;
538538

539-
const LEGAL_TEXT = 'Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here\'s the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry\'s choice of terms.';
539+
// Derived from the template to stay in sync automatically
540+
const LEGAL_BOILERPLATE_SECTION = extractLegalBoilerplateSection(PR_TEMPLATE_WITH_BOILERPLATE);
541+
const LEGAL_TEXT = LEGAL_BOILERPLATE_SECTION.replace('### Legal Boilerplate\n', '');
540542

541-
/**
542-
* Build a mock danger context for checkLegalBoilerplate tests.
543-
* @param {object} overrides - Properties to override on danger.github.pr
544-
* @param {string|null} templateContent - Content returned by fileContents for template paths (null = no template)
545-
*/
546543
function buildMockContext({ prOverrides = {}, templateContent = PR_TEMPLATE_WITH_BOILERPLATE } = {}) {
547544
const failMessages = [];
548545
const markdownMessages = [];
@@ -556,7 +553,6 @@ function buildMockContext({ prOverrides = {}, templateContent = PR_TEMPLATE_WITH
556553
},
557554
utils: {
558555
fileContents: async (path) => {
559-
// Only return content for the first standard template path
560556
if (templateContent && path === '.github/PULL_REQUEST_TEMPLATE.md') {
561557
return templateContent;
562558
}

danger/dangerfile.js

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
const { getFlavorConfig, extractPRFlavor, extractLegalBoilerplateSection, checkLegalBoilerplate } = require('./dangerfile-utils.js');
1+
const { getFlavorConfig, extractPRFlavor, checkLegalBoilerplate } = require('./dangerfile-utils.js');
22

33
const headRepoName = danger.github.pr.head.repo.git_url;
44
const baseRepoName = danger.github.pr.base.repo.git_url;
@@ -186,10 +186,6 @@ async function checkActionsArePinned() {
186186
}
187187
}
188188

189-
async function checkLegalBoilerplateRule() {
190-
await checkLegalBoilerplate({ danger, fail, markdown });
191-
}
192-
193189
async function checkFromExternalChecks() {
194190
// Get the external dangerfile path from environment variable (passed via workflow input)
195191
// Priority: EXTRA_DANGERFILE (absolute path) -> EXTRA_DANGERFILE_INPUT (relative path)
@@ -235,7 +231,7 @@ async function checkAll() {
235231
await checkDocs();
236232
await checkChangelog();
237233
await checkActionsArePinned();
238-
await checkLegalBoilerplateRule();
234+
await checkLegalBoilerplate({ danger, fail, markdown });
239235
await checkFromExternalChecks();
240236
}
241237

0 commit comments

Comments
 (0)