Skip to content

Commit

Permalink
Clean up git email validation.
Browse files Browse the repository at this point in the history
  • Loading branch information
iclanton committed Mar 30, 2024
1 parent 8f2fc47 commit ebe2934
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 33 deletions.
61 changes: 31 additions & 30 deletions libraries/rush-lib/src/logic/Git.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,18 +89,6 @@ export class Git {
}
}

/**
* If a Git email address is configured and is nonempty, this returns it.
* Otherwise, undefined is returned.
*/
public async tryGetGitEmailAsync(): Promise<string | undefined> {
const emailResult: IResultOrError<string> = await this._tryGetGitEmailAsync();
if (emailResult.result !== undefined && emailResult.result.length > 0) {
return emailResult.result;
}
return undefined;
}

/**
* If a Git email address is configured and is nonempty, this returns it.
* Otherwise, configuration instructions are printed to the console,
Expand All @@ -109,22 +97,16 @@ export class Git {
public async getGitEmailAsync(): Promise<string> {
// Determine the user's account
// Ex: "bob@example.com"
const emailResult: IResultOrError<string> = await this._tryGetGitEmailAsync();
if (emailResult.error) {
// eslint-disable-next-line no-console
console.log(
[
`Error: ${emailResult.error.message}`,
'Unable to determine your Git configuration using this command:',
'',
' git config user.email',
''
].join('\n')
);
throw new AlreadyReportedError();
}
const emailResult: string | undefined = await this.tryGetGitEmailAsync();
return this.validateGitEmail(emailResult);
}

if (emailResult.result === undefined || emailResult.result.length === 0) {
/**
* If the Git email address is configured and non-empty, this returns it. Otherwise
* it prints an error message and throws.
*/
public validateGitEmail(userEmail: string | undefined): string {
if (userEmail === undefined || userEmail.length === 0) {
// eslint-disable-next-line no-console
console.log(
[
Expand All @@ -139,7 +121,7 @@ export class Git {
throw new AlreadyReportedError();
}

return emailResult.result;
return userEmail;
}

/**
Expand Down Expand Up @@ -518,7 +500,11 @@ export class Git {
return result;
}

private async _tryGetGitEmailAsync(): Promise<IResultOrError<string>> {
/**
* Returns an object containing either the result of the `git config user.email`
* command or an error.
*/
public async tryGetGitEmailAsync(): Promise<string | undefined> {
if (this._gitEmailResult === undefined) {
const gitPath: string = this.getGitPathOrThrow();
try {
Expand All @@ -534,7 +520,22 @@ export class Git {
}
}

return this._gitEmailResult;
const { error, result } = this._gitEmailResult;
if (error) {
// eslint-disable-next-line no-console
console.log(
[
`Error: ${error.message}`,
'Unable to determine your Git configuration using this command:',
'',
' git config user.email',
''
].join('\n')
);
throw new AlreadyReportedError();
}

return result;
}

private async _tryGetGitHooksPathAsync(): Promise<IResultOrError<string>> {
Expand Down
6 changes: 3 additions & 3 deletions libraries/rush-lib/src/logic/policy/GitEmailPolicy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,20 +34,20 @@ export async function validateAsync(
return;
}

let userEmail: string | undefined = await git.tryGetGitEmailAsync();
// If there isn't a Git policy, then we don't care whether the person configured
// a Git email address at all. This helps people who don't
if (rushConfiguration.gitAllowedEmailRegExps.length === 0) {
if (git.tryGetGitEmailAsync() === undefined) {
if (userEmail === undefined) {
return;
}

// Otherwise, if an email *is* configured at all, then we still perform the basic
// sanity checks (e.g. no spaces in the address).
}

let userEmail: string;
try {
userEmail = await git.getGitEmailAsync();
userEmail = git.validateGitEmail(userEmail);

// sanity check; a valid email should not contain any whitespace
// if this fails, then we have another issue to report
Expand Down

0 comments on commit ebe2934

Please sign in to comment.