Fix istanbul-lib-coverage merge when two branch statements have the same first location

[JonathanGawrych]

Hello! We are experiencing an issue using merge when our coverage's branch map contains the same first location. Take for example this code:

	public isInAsyncSlideEditMode (): boolean {
		return this.playerSync?.isCaptureMode() &&
			this.sourceMedia &&
			this.sourceMedia.isDocument() &&
			!this.activity.isLiveSessionEnabled() &&
			this.activity.isSlidesEnabled();
	};

This generated this branch map:

"branchMap": {
	"0": {
		"loc": {
			"start": {
				"line": 59,
				"column": 9
			},
			"end": {
				"line": 59,
				"column": null
			}
		},
		"type": "binary-expr",
		"locations": [
			{
				"start": {
					"line": 59,
					"column": 9
				},
				"end": {
					"line": 59,
					"column": null
				}
			},
			{
				"start": {
					"line": 60,
					"column": 3
				},
				"end": {
					"line": 60,
					"column": null
				}
			},
			{
				"start": {
					"line": 61,
					"column": 3
				},
				"end": {
					"line": 61,
					"column": null
				}
			},
			{
				"start": {
					"line": 62,
					"column": 3
				},
				"end": {
					"line": 62,
					"column": null
				}
			},
			{
				"start": {
					"line": 63,
					"column": 3
				},
				"end": {
					"line": 63,
					"column": null
				}
			}
		]
	},
	"1": {
		"loc": {
			"start": {
				"line": 59,
				"column": 24
			},
			"end": {
				"line": 59,
				"column": null
			}
		},
		"type": "cond-expr",
		"locations": [
			{
				"start": {
					"line": 59,
					"column": 24
				},
				"end": {
					"line": 59,
					"column": null
				}
			},
			{
				"start": {
					"line": 59,
					"column": 24
				},
				"end": {
					"line": 59,
					"column": null
				}
			}
		]
	},
	"2": {
		"loc": {
			"start": {
				"line": 59,
				"column": 9
			},
			"end": {
				"line": 59,
				"column": null
			}
		},
		"type": "binary-expr",
		"locations": [
			{
				"start": {
					"line": 59,
					"column": 9
				},
				"end": {
					"line": 59,
					"column": null
				}
			},
			{
				"start": {
					"line": 59,
					"column": 24
				},
				"end": {
					"line": 59,
					"column": null
				}
			}
		]
	},
	"3": {
		"loc": {
			"start": {
				"line": 67,
				"column": 10
			},
			"end": {
				"line": 67,
				"column": 26
			}
		},
		"type": "binary-expr",
		"locations": [
			{
				"start": {
					"line": 67,
					"column": 10
				},
				"end": {
					"line": 67,
					"column": 26
				}
			},
			{
				"start": {
					"line": 67,
					"column": 26
				},
				"end": {
					"line": 67,
					"column": null
				}
			}
		]
	}
}

Note that branchMap[0].locations[0] is the exact same as branchMap[2].locations[0]. During merge it uses location[0] to generate the key to uniquely identify each branch, but these two have the same key. The result is the merge losing 2 branch, so instead of a total 11, it's a total of 9. Additionally depending on the order of the merge, we get a total number of coverage branches to be randomly 6 or 9.

This change makes the key from the entire location object, so these two branches are now considered different and are not munged together during merge.

Thanks!

[erikdonohoo]

This is really causing us a headache in our automated pipelines for coverage tracking as it flips all over the place.

[bcoe]

@JonathanGawrych doesn't this make the keyFromLocationsProp method basically the same as keyFromLoc?

const keyFromLoc = ({ start, end }) =>
    `${start.line}|${start.column}|${end.line}|${end.column}`;

Any idea what the reason for these two different methods was in the first place? I think I would need to dig a bit deeper to understand.

[JonathanGawrych]

They would be slightly different, as the .map() would execute keyFromLoc on each entry of the location, rather than just the first one. For example, take a look at my example branchMap. Before this code change, the keys generated would be:

• branchMap[0] => "59|9|59|null"
• branchMap[1] => "59|24|59|null"
• branchMap[2] => "59|9|59|null"
• branchMap[3] => "67|10|67|26"

Notice how branchMap[0] and branchMap[2] are the same. Because the keys aren't unique, the merge doesn't work properly and munges those two branches together.

After this code change, the keys generated would use the entire location array:

• branchMap[0] => "59|9|59|null;60|3|60|null;61|3|61|null;62|3|62|null;63|3|63|null"
• branchMap[1] => "59|24|59|null;59|24|59;null"
• branchMap[2] => "59|9|59|null;59|24|59|null"
• branchMap[3] => "67|10|67|26;67|26|67|null"

Now branchMap[0] and branchMap[2] are different. I'm not sure why the original code only used the first location. The current code does work in 99% of cases, because normally it's impossible for two branches to start in the same location. However in the 1% of cases where code is transpiled such that the source mapping makes two branches start in the same location, this fixes that problem.