feat(locale): add hyphenated names to English locales

[matthewmayer]

fix #691

• default en locale gets 95% non-hyphenated 5% hyphenated
• other locales without existing last_name_patterns get 100% non-hyphenated
• locales with split male_last_name and female_last_names get split patterns
• existing locales with last_name_patterns (e.g. lv, en_GB) are not changed.

If additional information about hyphenation rules in other locales can be found, this can be added in a subsequent PR. I kept this one formulaic to make it easy to review.

[codecov]

Codecov Report

Merging #1872 (620d6cc) into next (8a4ca4e) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

❗ Current head 620d6cc differs from pull request most recent head 74631cc. Consider uploading reports for the commit 74631cc to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             next    #1872      +/-   ##
==========================================
- Coverage   99.64%   99.62%   -0.02%     
==========================================
  Files        2384     2450      +66     
  Lines      238992   239274     +282     
  Branches     1231     1228       -3     
==========================================
+ Hits       238132   238387     +255     
- Misses        838      865      +27     
  Partials       22       22              

Impacted Files	Coverage Δ
src/locales/af_ZA/person/index.ts	100.00% <100.00%> (ø)
src/locales/af_ZA/person/last_name_patterns.ts	100.00% <100.00%> (ø)
src/locales/ar/person/index.ts	100.00% <100.00%> (ø)
src/locales/ar/person/last_name_patterns.ts	100.00% <100.00%> (ø)
src/locales/az/person/female_last_name_patterns.ts	100.00% <100.00%> (ø)
src/locales/az/person/index.ts	100.00% <100.00%> (ø)
src/locales/az/person/male_last_name_patterns.ts	100.00% <100.00%> (ø)
src/locales/cz/person/female_last_name_patterns.ts	100.00% <100.00%> (ø)
src/locales/cz/person/index.ts	100.00% <100.00%> (ø)
src/locales/cz/person/male_last_name_patterns.ts	100.00% <100.00%> (ø)
... and 108 more

... and 3 files with indirect coverage changes

[matthewmayer]

this exposed a bug in the test for commitEntry, the local part of the email addresses was not allowing hyphens. i changed the regex to match the one we use in the emailAddress method https://github.com/faker-js/faker/blob/add2254ca31434af3003c0051064bdb27713992d/src/modules/internet/index.ts

[Shinigami92]

this exposed a bug in the test for commitEntry, the local part of the email addresses was not allowing hyphens. i changed the regex to match the one we use in the emailAddress method add2254/src/modules/internet/index.ts

Please lets extract this to its own PR, so

1. it is reviewable in parallel to this PR
2. it gets its own "fix/test" changelog entry

[matthewmayer]

I considered that, but the test doesn't fail on the next branch because email addresses in en never contain hyphens.

[Shinigami92]

I considered that, but the test doesn't fail on the next branch because email addresses in en never contain hyphens.

You are free to add static tests that handles special cases in the random seeded test cases (second part of each test file)

Maybe we need to change something here instead of the test

faker/src/modules/git/index.ts

Line 123 in 11a5f51

user = user.replace(/^[\.,:;"\\']|[\<\>\n]|[\.,:;"\\']$/g, '');

We are already starting to pollute this PR with unrelated discussions, please lets stop this and extract it so we can have dedicated discussions

[matthewmayer]

I considered that, but the test doesn't fail on the next branch because email addresses in en never contain hyphens.

You are free to add static tests that handles special cases in the random seeded test cases (second part of each test file)

Maybe we need to change something here instead of the test

faker/src/modules/git/index.ts

Line 123 in 11a5f51

user = user.replace(/^[\.,:;"\\']|[\<\>\n]|[\.,:;"\\']$/g, '');

We are already starting to pollute this PR with unrelated discussions, please lets stop this and extract it so we can have dedicated discussions

#1876

[ST-DDT]

These lines are now somewhat mandatory:

faker/src/modules/person/index.ts

Lines 120 to 135 in 11a5f51

	if (
	last_name_patterns != null ||
	male_last_name_patterns != null ||
	female_last_name_patterns != null
	) {
	const pattern = selectDefinition(
	this.faker,
	this.faker.helpers.weightedArrayElement,
	sex,
	{
	generic: last_name_patterns,
	female: female_last_name_patterns,
	male: male_last_name_patterns,
	}
	);
	return this.faker.helpers.fake(pattern);

So IMO the other code branch could be removed.
Also the PR does more than the PR title suggests IMO.

Not sure whether it is worth splitting into two PRs:

1. Make last_name_patterns required/Generate last_name_patterns for all locales
2. add hyphenated names to English locales

[matthewmayer]

It's not mandatory if you create a new locale which doesn't extend en.

[ST-DDT]

Please fix the merge conflicts.

[ST-DDT]

Please adjust the PR title to reflect the current. It also changes non English locales.

[matthewmayer]

Please adjust the PR title to reflect the current. It also changes non English locales.

it doesn't add any hyphenated patterns to any non-English locales, the changes are to prevent hyphenated patterns from propagating to the other non-English locales unexpectedly.

[matthewmayer]

could append ", add last_name_patterns to all locales" maybe? but makes it a bit long