Doesn't handle embedded comments

[chocolateboy]

There are several issues with the regex:

• It doesn't handle embedded comments
• It considers functionfoo () { } to be a valid function declaration
• Nit: \{([\w\W\s\S]*)\} is a roundabout way of saying \{(.*)\} (though I'd go with \{(.*?)\})

test

var re = require('function-regex')();

function /* 1 */ test /* 2 */ (/* 3 */ foo /* 4 */ ) /* 5 */ {
    return foo;
}

var match1 = re.exec(test.toString());
var match2 = re.exec('functionfoo(){}');

console.log("should match:", match1); // should match but doesn't
console.log("shouldn't match:", match2); // shouldn't match but does

output:

should match: null
shouldn't match: [ 'functionfoo(){}',
  'foo',
  '',
  '',
  index: 0,
  input: 'functionfoo(){}' ]

[jonschlinkert]

hmm, seems like a reasonable expectation. also seems like comment matching or stripping would be beyond the scope of this though. not sure what others think. perhaps we can add something to the readme that suggests that users strip comments first?

[tunnckoCore]

also seems like comment matching or stripping would be beyond the scope of this though ... perhaps we can add something to the readme that suggests that users strip comments first?

👍

[tunnckoCore]

I think the only thing that we should change is first \s* to \s+ because the

var match2 = re.exec('functionfoo(){}');

use case

[tunnckoCore]

I think the only thing that we should change is first \s* to \s+

haha, but it's intentional.. lol because the function() {} case.

[blakeembrey]

@tunnckoCore Off the top of my head, use: (?:\s+[\w$][\w\d$]*)?\s*

E.g. /^function(?:\s+[\w$][\w\d$]*)?\s*\(([\w\s,$]*)\)\s*\{([\w\W\s\S]*)\}$/

[tunnckoCore]

@blakeembrey Hmm.. nope exactly.
@chocolateboy http://regexr.com/3albo what you think?

More use cases?

[tunnckoCore]

Always.. regexes are not enough.

[chocolateboy]

Always.. regexes not enough.

I can't think of any reason why a regex shouldn't work (and correctly handle comments) for ES5 functions, but it won't work for ES6 functions:

A regex will never be able to handle destructured parameters in the general case, since regular languages don't support balanced delimiters.

[chocolateboy]

I was wrong about the "nit". I'd forgotten that JavaScript doesn't provide a way to make . match newline. Still, [\s\S] matches everything and [\w\W] matches everything, so only one is needed.

[chocolateboy]

Something like this should work (though note the parameter list includes comments):

^ function (?: space+ (ident) )? space* \( space* (params)? space* \) space* \{ (body) \} $

e.g.:

generate

function /* 1 */ test /* 2 */ ( /* 3 */ foo /* 4 */ , /* 5 */ bar /* 6 */ ) /* 7 */ {
    return foo;
}

function str (re) {
    return re.toString().slice(1, -1);
}

var fn     = str(/^ function (?: space+ (ident) )? space* \( space* (params)? space* \) space* \{ (body) \} $ /);
var params = str(/ ident (?: space* , space* ident )* /);
var space  = str(/ (?: (?: \s+ ) | (?: \/ \* [\s\S]*? \* \/ ) | (?: \/\/ [^\r\n]* ) ) /);
var ident  = str(/ (?: [a-zA-Z_$] [\w$]* ) /);
var body   = str(/ (?: [\s\S]*? ) /);

fn = fn.replace(/params/g, params)
    .replace(/space/g, space)
    .replace(/ident/g, ident)
    .replace(/body/g, body)
    .replace(/\s+/g, '');

var re = new RegExp(fn);

console.log("test: %j", test.toString());
console.log(re);
console.log(re.exec(test.toString()))

function

test: "function test( /* 3 */ foo /* 4 */ , /* 5 */ bar /* 6 */ ) /* 7 */ {\n    return foo;\n}"

regex

/^function(?:(?:(?:\s+)|(?:\/\*[\s\S]*?\*\/)|(?:\/\/[^\r\n]*))+((?:[a-zA-Z_$][\w$]*)))?(?:(?:\s+)|(?:\/\*[\s\S]*?\*\/)|(?:\/\/[^\r\n]*))*\((?:(?:\s+)|(?:\/\*[\s\S]*?\*\/)|(?:\/\/[^\r\n]*))*((?:[a-zA-Z_$][\w$]*)(?:(?:(?:\s+)|(?:\/\*[\s\S]*?\*\/)|(?:\/\/[^\r\n]*))*,(?:(?:\s+)|(?:\/\*[\s\S]*?\*\/)|(?:\/\/[^\r\n]*))*(?:[a-zA-Z_$][\w$]*))*)?(?:(?:\s+)|(?:\/\*[\s\S]*?\*\/)|(?:\/\/[^\r\n]*))*\)(?:(?:\s+)|(?:\/\*[\s\S]*?\*\/)|(?:\/\/[^\r\n]*))*\{((?:[\s\S]*?))\}$/

match

[ 'function test( /* 3 */ foo /* 4 */ , /* 5 */ bar /* 6 */ ) /* 7 */ {\n    return foo;\n}',
  'test',
  'foo /* 4 */ , /* 5 */ bar',
  '\n    return foo;\n',
  index: 0,
  input: 'function test( /* 3 */ foo /* 4 */ , /* 5 */ bar /* 6 */ ) /* 7 */ {\n    return foo;\n}' ]

[tunnckoCore]

Hm. Looks good, but what about /* 3 */ and /* 6 */ comments? If we will accept comments and include them in response why only these in the middle?

[chocolateboy]

what about /* 3 / and / 6 */ comments?

The regex originally included them. Put them back in if you want them :-)

var fn     = str(/^ function (?: space+ (ident) )? space* \( (params)? \) space* \{ (body) \} $ /);
var params = str(/ space* ident (?: space* , space* ident )* space* /);

[ 'function test( /* 3 */ foo /* 4 */ , /* 5 */ bar /* 6 */ ) /* 7 */ {\n    return foo;\n}',
  'test',
  ' /* 3 */ foo /* 4 */ , /* 5 */ bar /* 6 */ ',
  '\n    return foo;\n',
  index: 0,
  input: 'function test( /* 3 */ foo /* 4 */ , /* 5 */ bar /* 6 */ ) /* 7 */ {\n    return foo;\n}' ]

[tunnckoCore]

Hm, good but I keep thinking of that

also seems like comment matching or stripping would be beyond the scope of this though

we already have libs for stripping comments, so its normal first to strip them after that using this regex.

I dont know, others @regexhq/owners ?

[tunnckoCore]

but we should only handle this case 'functionfoo(){}'

[chocolateboy]

but we should only handle this case 'functionfoo(){}'

That's already handled by this regex, as well as other fixes e.g.:

current regex

// invalid identifiers: shouldn't match, but does
require('function-regex')().exec("function 1 (2, 3, 4) { }")

[ 'function 1 (2, 3, 4) { }',
  '1',
  '2, 3, 4',
  ' ',
  index: 0,
  input: 'function 1 (2, 3, 4) { }' ]

this regex

// invalid identifiers: doesn't match
re.exec("function 1 (2, 3, 4) { }")

null

At any rate, this approach is designed to be easy to read, understand and modify, which is more than can be said for hacking on the regex directly. If you're happy for this regex to break on comments, you can just remove them from the space pattern:

var space = str(/ (?: \s+ ) /);