Add a presubmit check that catches |const NSString*| and friends.

Add a presubmit check that catches |const NSString*| and friends.

BUG=TEST=none

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=54925

[Nico, 2010-08-03 22:18:16]

Jeremy asked for this long, long ago. I should've listened back then, would've
saved me some work of removing instances of this that have reappeared.

[jeremy, 2010-08-03 22:27:22]

Didn't check style, some minor comments.

http://codereview.chromium.org/3082016/diff/1/2
File PRESUBMIT.py (right):

http://codereview.chromium.org/3082016/diff/1/2#newcode36
PRESUBMIT.py:36: pattern = re.compile(r'const NS\w*\ ?\*')
Slightly more inclusive regex:
const\s+NS

?

http://codereview.chromium.org/3082016/diff/1/2#newcode39
PRESUBMIT.py:39: if f.LocalPath().endswith('.h') or
f.LocalPath().endswith('.mm'):
Does it make sense to check .m files?

http://codereview.chromium.org/3082016/diff/1/2#newcode49
PRESUBMIT.py:49: return [ res_type('|const NSClass*| is wrong, see
http://bit.ly/a4ngN0',
That link points to a message on the mailing list which then goes on to point to
a code review, why not put the relevant information on our wiki and point there
directly or point at the code review?

[M-A Ruel, 2010-08-04 00:24:26]

I don't think BUG=TEST=none works.

http://codereview.chromium.org/3082016/diff/3001/4001
File PRESUBMIT.py (right):

http://codereview.chromium.org/3082016/diff/3001/4001#newcode11
PRESUBMIT.py:11: import re
remove, use input_api.re instead.

http://codereview.chromium.org/3082016/diff/3001/4001#newcode34
PRESUBMIT.py:34: def _CheckConstNSObject(input_api, output_api):
def _CheckConstNSObject(input_api, output_api, source_filter):

http://codereview.chromium.org/3082016/diff/3001/4001#newcode36
PRESUBMIT.py:36: pattern = re.compile(r'const NS\w*\ ?\*')
use 'input_api.re' instead of 're'.

http://codereview.chromium.org/3082016/diff/3001/4001#newcode38
PRESUBMIT.py:38: for f in input_api.change.AffectedTextFiles():
Use instead input_api.AffectedSourceFiles(source_filter)

http://codereview.chromium.org/3082016/diff/3001/4001#newcode49
PRESUBMIT.py:49: return [ res_type('|const NSClass*| is wrong, see
http://bit.ly/a4ngN0',
please don't use bit.ly here

http://codereview.chromium.org/3082016/diff/3001/4001#newcode82
PRESUBMIT.py:82: results.extend(_CheckConstNSObject(input_api, output_api))
results.extend(_CheckConstNSObject(input_api, output_api,
source_file_filter=sources))

[Nico, 2010-08-04 04:46:59]

thanks, guys!

http://codereview.chromium.org/3082016/diff/1/2
File PRESUBMIT.py (right):

http://codereview.chromium.org/3082016/diff/1/2#newcode36
PRESUBMIT.py:36: pattern = re.compile(r'const NS\w*\ ?\*')
On 2010/08/03 22:27:22, jeremy wrote:
> Slightly more inclusive regex:
> const\s+NS
> 
> ?

Shouldn't be needed, but why not :-) Done.

http://codereview.chromium.org/3082016/diff/1/2#newcode39
PRESUBMIT.py:39: if f.LocalPath().endswith('.h') or
f.LocalPath().endswith('.mm'):
On 2010/08/03 22:27:22, jeremy wrote:
> Does it make sense to check .m files?

I don't think we have .m files.

http://codereview.chromium.org/3082016/diff/1/2#newcode49
PRESUBMIT.py:49: return [ res_type('|const NSClass*| is wrong, see
http://bit.ly/a4ngN0',
On 2010/08/03 22:27:22, jeremy wrote:
> That link points to a message on the mailing list which then goes on to point
to
> a code review, why not put the relevant information on our wiki and point
there
> directly or point at the code review?

Done.

http://codereview.chromium.org/3082016/diff/1/2#newcode49
PRESUBMIT.py:49: return [ res_type('|const NSClass*| is wrong, see
http://bit.ly/a4ngN0',
On 2010/08/03 22:27:22, jeremy wrote:
> That link points to a message on the mailing list which then goes on to point
to
> a code review, why not put the relevant information on our wiki and point
there
> directly or point at the code review?

Done.

http://codereview.chromium.org/3082016/diff/3001/4001#newcode11
PRESUBMIT.py:11: import re
On 2010/08/04 00:24:26, Marc-Antoine Ruel wrote:
> remove, use input_api.re instead.

Done.

(out of interest: why?)

http://codereview.chromium.org/3082016/diff/3001/4001#newcode34
PRESUBMIT.py:34: def _CheckConstNSObject(input_api, output_api):
On 2010/08/04 00:24:26, Marc-Antoine Ruel wrote:
> def _CheckConstNSObject(input_api, output_api, source_filter):

Done.

http://codereview.chromium.org/3082016/diff/3001/4001#newcode36
PRESUBMIT.py:36: pattern = re.compile(r'const NS\w*\ ?\*')
On 2010/08/04 00:24:26, Marc-Antoine Ruel wrote:
> use 'input_api.re' instead of 're'.

Done.

http://codereview.chromium.org/3082016/diff/3001/4001#newcode38
PRESUBMIT.py:38: for f in input_api.change.AffectedTextFiles():
On 2010/08/04 00:24:26, Marc-Antoine Ruel wrote:
> Use instead input_api.AffectedSourceFiles(source_filter)

Done.

http://codereview.chromium.org/3082016/diff/3001/4001#newcode38
PRESUBMIT.py:38: for f in input_api.change.AffectedTextFiles():
On 2010/08/04 00:24:26, Marc-Antoine Ruel wrote:
> Use instead input_api.AffectedSourceFiles(source_filter)

Done.

http://codereview.chromium.org/3082016/diff/3001/4001#newcode49
PRESUBMIT.py:49: return [ res_type('|const NSClass*| is wrong, see
http://bit.ly/a4ngN0',
On 2010/08/04 00:24:26, Marc-Antoine Ruel wrote:
> please don't use bit.ly here

Done.

[M-A Ruel, 2010-08-04 13:47:44]

lgtm

http://codereview.chromium.org/3082016/diff/3001/4001
File PRESUBMIT.py (right):

http://codereview.chromium.org/3082016/diff/3001/4001#newcode11
PRESUBMIT.py:11: import re
On 2010/08/04 04:46:59, Nico wrote:
> On 2010/08/04 00:24:26, Marc-Antoine Ruel wrote:
> > remove, use input_api.re instead.
> 
> Done.
> 
> (out of interest: why?)

PRESUBMIT.py are self-contained and don't import anything. I didn't make that
design decision but keeping it for consistency.

[jeremy, 2010-08-04 16:42:34]

LGTM

[Nico, 2010-08-04 17:44:52]

http://codereview.chromium.org/3082016/diff/10001/11001
File PRESUBMIT.py (right):

http://codereview.chromium.org/3082016/diff/10001/11001#newcode39
PRESUBMIT.py:39: if pattern.match(contents):
(turns out this needs to be |.search|, not |.match|)