This is an archive of the discontinued Mercurial Phabricator instance.

Differential D2866

contrib: fix a subtle bug in check-code's regex rewriting
ClosedPublic

Authored by durin42 on Mar 14 2018, 3:53 PM.

Download Raw Diff

Details

Reviewers

pulkit

Group Reviewers

hg-reviewers

Commits

rHGa8d540d2628c: contrib: fix a subtle bug in check-code's regex rewriting
rHGac635637ed2f: contrib: fix a subtle bug in check-code's regex rewriting

Summary

We rewrite \s to [ \t] when preparing our regular expressions, but
we previously weren't working to avoid having nested sets. Previously,
Python let this slide without incident, but in Python 3.7 wants to
make sure you meant an actual [ in a set, and so this warns. This
appears to be fortunate for us, because [\s(] was getting rewritten
to be [[ \t](] which doesn't actually match what we expected. See
preceding changes that were revealed to be necessary after
implementing this fix.

Diff Detail

Repository

rHG Mercurial

Lint

Lint Skipped

Unit

Unit Tests Skipped

Event Timeline

durin42 created this revision.Mar 14 2018, 3:53 PM

Herald added a reviewer: hg-reviewers. · View Herald TranscriptMar 14 2018, 3:53 PM

Herald added a subscriber: mercurial-devel. · View Herald Transcript

Looks good to me but I am not feeling confident enough to push this. Queued the first three.

Closed by commit rHGac635637ed2f: contrib: fix a subtle bug in check-code's regex rewriting (authored by durin42). · Explain WhyMar 15 2018, 10:43 AM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			contrib/check-code.py (7 lines)

Status	Author	Revision
Closed	durin42	D2866 contrib: fix a subtle bug in check-code's regex rewriting
Closed	durin42	D2865 tests: clean up two wayward `python` invocations
Closed	durin42	D2864 contrib: open a hole in the open().read() ban for open().close()
Closed	durin42	D2863 cleanup: fix some latent open(path).read() et al calls we previously missed

Diff 7044

contrib/check-code.py


	def _preparepats():			def _preparepats():
	for c in checks:			for c in checks:
	failandwarn = c[-1]			failandwarn = c[-1]
	for pats in failandwarn:			for pats in failandwarn:
	for i, pseq in enumerate(pats):			for i, pseq in enumerate(pats):
	# fix-up regexes for multi-line searches			# fix-up regexes for multi-line searches
	p = pseq[0]			p = pseq[0]
	# \s doesn't match \n			# \s doesn't match \n (done in two steps)
	p = re.sub(r'(?<!\\)\\s', r'[ \\t]', p)			# first, we replace \s that appears in a set already
				p = re.sub(r'\[\\s', r'[ \\t', p)
				# now we replace other \s instances.
				p = re.sub(r'(?<!(\\\|\[))\\s', r'[ \\t]', p)
	# [^...] doesn't match newline			# [^...] doesn't match newline
	p = re.sub(r'(?<!\\)\[\^', r'[^\\n', p)			p = re.sub(r'(?<!\\)\[\^', r'[^\\n', p)

	pats[i] = (re.compile(p, re.MULTILINE),) + pseq[1:]			pats[i] = (re.compile(p, re.MULTILINE),) + pseq[1:]
	filters = c[3]			filters = c[3]
	for i, flt in enumerate(filters):			for i, flt in enumerate(filters):
	filters[i] = re.compile(flt[0]), flt[1]			filters[i] = re.compile(flt[0]), flt[1]

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	7044		Mar 14 2018, 3:53 PM	★	★
Diff 2	7066	rHGac635637ed2fa63c8daf70a437d88b656b04c5d6	Mar 14 2018, 2:05 PM	★	★