解析依赖时,移除代码注释产生的问题(内有提取依赖的优化活动) #478

Closed
ethanzheng opened this Issue Dec 18, 2012 · 108 comments
@ethanzheng

seajs在解释依赖模块时,先移除模块代码里的所有注释,然后在移除过注释的代码里查找require方法调用,以得到该模块的依赖项。

不过移除模块代码注释的时候,只用了一个replace方法,得到的结果代码有可能是不完整的:

src/util-deps.js

var COMMENT_RE = /(\/\*([\s\S]*?)\*\/|([^:]|^)\/\/(.*)$)/mg
code = code.replace(COMMENT_RE, '')

module code

var str = "//In face, this is not a comment."; require("module A");
require("module B");

调用完 code.replace(COMMENT_RE, '')后, 代码变成

var str = 
require("module B");

依赖项将缺少 "module A", 至少在chrome(23.0.1271.97 m)中的行为是这样的。

具体的结果依赖于浏览器的Function.prototype.toString 方法:
deps = util.parseDependencies(factory.toString())
在FF中,似乎会把代码格式化,toString后的代码不存在多条语句在同一行的情况,绕过了这个问题。别的浏览器暂时没有测试。

不知道这里是为了保持代码简单所以这样简单处理还是我忽略了某些历史原因?

这条issues昨天被我错误地发到了 spm项目那边去了,得知用spm解析依赖的时候不会有这个问题。

@lifesinger
seajs member

这个问题非常经典,可以见看测试页面:

http://seajs.org/tests/research/remove-comments-safely/test.html

特别注意测试代码里有两行:

var s = '// i am string'; require('xx');
var t = 'i am string too'; // require('zz');

目前比较成熟的有两种方案:

方案A

code.replace(/(\/\*([\s\S]*?)\*\/|([^:]|^)\/\/(.*)$)/mg, '')

方案B

// multi-line comments
code = code.replace(/^\s*\/\*[\s\S]*?\*\/\s*$/mg, '')

// single-line comments
code = code.replace(/^\s*\/\/.*$/mg, '')

SeaJS 最开始采用的是方案B,方案B可以确保不会误杀注释,上面的两行测试代码,经过替换后,会保持不变:

var s = '// i am string'; require('xx');
var t = 'i am string too'; // require('zz');

方案B带来的问题是,依赖里会多出 zz

为了解决方案B的问题,于是 SeaJS 调整策略,采用了更为激烈的注释除去方案,也就是方案A,在方案A下,经过替换后,测试代码会变成:

var s = 
var t = 'i am string too';

显然,方案A在解决方案B的问题时,也带来了新问题:依赖里会少了 xx

除了方案A和方案B,还有各种正则方案,但无一例外,都无法做到完全没有 bug,甚至可以说:

想通过纯正则来除去 JavaScript 的代码注释是一件不可能的事情

真想实现的话,得利用更复杂方案,比如基于 JavaScript 的 Compiler,得到 AST,通过 AST 来去除注释,但这样,对 SeaJS 来说,无疑增加了巨大的复杂度。这就如我们去餐馆吃饭,却被建议先得去一趟筷子厂自己做双筷子一样。

于是回到 SeaJS 本身的需求,在 SeaJS 的这段代码里,去除注释,仅仅是为了更好的拿到依赖信息。因此使用正则是合理的方式,这比较简洁。

对于选择方案A还是方案B,则取决于他们对应的场景那个更常见,也就是在实际代码中

场景A

var t = 'i am string too'; // require('zz');

场景B

var s = '// i am string'; require('xx');

场景A和场景B哪个更会出现?

从目前我接触到的实际项目代码来看,这两个场景都不常见,相对而言,场景A出现的概率更高,经常出现在原本代码是:

var t = require('zz')

但为了调试等因素,会临时修改成:

var t = { mock object }; // require('zz')

因此考虑场景A出现的概率大于场景B,因此 SeaJS 里目前采用了对应的方案A。

@lifesinger lifesinger closed this Dec 18, 2012
@lifesinger
seajs member

通过正则,也有可能会有很简洁的方式可以实现 JavaScript 代码注释的安全正确去除。
目前这个 issue 也提交给 RequireJS 了: requirejs/requirejs#542

有兴趣的可以挑战下,或许能找到合适又简洁的方案。
特此悬赏,有挑战成功者,送 100 美刀现金。

@xingrz

想到了一个思路,不知是否可行:

  1. 匹配出所有字符串(也就是''或者""包裹的东西),把它们替换成唯一的随机字符串(只包含A-Za-z0-9),在一个Object中储存这个唯一字符串和原字符串的对应关系
  2. 用正则移除注释(//之后的或/**/之间的内容)
  3. 找出requre,用中间的字符串(也就是第1步被我替换掉的)在第1步的Object中找到原始的字符串,就是它的依赖了
@bakso

mark留名 相信可以通过正则解决~

@czy88840616

正则之前想了很久,无果。。现在用的http://james.padolsey.com/javascript/removing-comments-in-javascript/ 这位的方案..

@lifesinger
seajs member

补充挑战条件:

  1. 纯原生代码,可以调用 ES5 中的 forEach 等方法。
  2. 有效代码行数不能超过 10 行,每行不能超过 120 个字符。
  3. 可以跑过这个例子 http://seajs.org/tests/research/remove-comments-safely/test.html
  4. 替换掉 SeaJS 的 parseDependencies 方法后,SeaJS 的 test cases 可保持全部通过。
  5. 性能不比 James 的 removeComments4 差:http://jsperf.com/remove-comments
  6. 不用考虑 var a=/ // /b\//g; 这种极端情况。
@lifesinger
seajs member

@czy88840616 James 的那个方案依旧有问题,可以见回复。

@dotnil

James 那个方案正是我想尝试的…… 我觉得除了解析完整的 AST,自己搞个简陋的状态机是最容易的了

@iwillwen

留名,回家挑战下

@czy88840616

@lifesinger 额,的确。。james后来又post了一个进阶版http://james.padolsey.com/javascript/javascript-comment-removal-revisted/ 不过依旧有些不足

@army8735
seajs member

这种情况考虑过么:/'\/require('mod')//*/

@army8735
seajs member

写错了:/\//require('mod')/*/

@lifesinger
seajs member

@army8735 这个针对 seajs 的依赖提取需求,其实没问题,被干掉了,也就不会多出依赖来,呵呵

@army8735
seajs member

那反过来吧,字符串中的require和正则中的require去掉了吗?@lifesinger

@lifesinger
seajs member

@army8735 没,这个好像也蛮麻烦的

@cuixiping

麻烦的只有正则,特别是除号和正则符的判断较难。
赞同 自己搞个简陋的状态机是最容易的了

@lifesinger
seajs member

还有一个权衡点是性能,James 那个其实满足需求,修改修改,也能安全去除掉字符串和正则中的 require,但性能有点难以接受:http://jsperf.com/remove-comments 最慢的那个

@lifesinger
seajs member

@cuixiping 除号和正则的情况可以排除掉,挑战条件我更新了下:

补充挑战条件:

  1. 纯原生代码,可以调用 ES5 中的 forEach 等方法。
  2. 有效代码行数不能超过 10 行,每行不能超过 120 个字符。
  3. 可以跑过这个例子 http://seajs.org/tests/research/remove-comments-safely/test.html
  4. 替换掉 SeaJS 的 parseDependencies 方法后,SeaJS 的 test cases 可保持全部通过。
  5. 性能不比 James 的 removeComments4 差:http://jsperf.com/remove-comments
  6. 不用考虑 var a=/ // /b\//g; 这种极端情况。
@army8735
seajs member

我先弄个除了5性能那点其它全部涵盖的版本,优化再看看吧。

@army8735
seajs member

不厚道:
function p(e){function f(){b=e.charAt(a++)}function j(){return"_"==b||"$"==b||"."==b||"A"<=b&&"z">=b||0<=b&&9>=b}for(var a=0,b,g=e.length,c=!0,h=!1,d=[0],i="";a<g;)if(f(),"\n"==b)c=!0;else if(!/\s/.test(b))if('"'==b||"'"==b){h&&d.push(a-1);for(c=b;a<g;)if(f(),"\"==b)a++;else if(b==c)break;h&&d.push(a);c=!0}else if("/"==b)if(f(),"/"==b)d.push(a-2),a=e.indexOf("\n",a),-1==a&&(a=e.length),d.push(a),c=!0;else if(""==b)d.push(a-2),a=e.indexOf("/",a)+2,d.push(a),c=!0;else if(c){a--;for(d.push(a-1);a<
g;)if(f(),"\"==b)a++;else if("/"==b)break;else if("["==b)for(;;)if(f(),"\"==b)a++;else if("]"==b)break;d.push(a);c=!1}else c=!0;else if(j()){a:for(c=a-1;a<g;)if(f(),!j()){h="require"!=e.slice(c,--a);break a}c=!1}else c=!0;d.push(g);for(a=0;a<d.length;a+=2)i+=e.slice(d[a],d[a+1]);return i}

@chocoboboy

失败啊...完全不合要求

function removeComments3(codeStr){
  var stateDef = { normal:0, singleLineComment:1, multiLineComment:2, expression:3, doubleQuotString:4, singleQuotString: 5,slash: 6 };
  var state = 0; result=[];  
  for (var i = 0; i < codeStr.length; i ++){
    var c = codeStr[i];
    switch(state){
      case stateDef.normal:
        switch(c){
          case "'": state=stateDef.singleQuotString;break;
          case '"': state=stateDef.doubleQuotString;break;
          case '/': state=stateDef.slash;break;
        }
        result.push(c);
      break;
      case stateDef.slash:
        switch(c){
          case '/': 
            state=stateDef.singleLineComment;
            result=result.splice(0,result.length-1);
            break;
          case '*': 
            state=stateDef.multiLineComment;
            result=result.splice(0,result.length-1);
            break;
          default: 
            state=stateDef.expression;
            result.push(c);
        }       
      break;
      case stateDef.singleLineComment:
        switch(c){
          case "\r": 
          case "\n": 
            state=stateDef.normal; 
            result.push(c);
            break;         
        }        
      break;
      case stateDef.multiLineComment:
        if(c==='*' && codeStr[i+=1]==='/') { state=stateDef.normal;}
      break;
      case stateDef.expression:
        if(c===';' || c==='\r' || c==='\n') { state=stateDef.normal;}
        result.push(c);
      break;
      case stateDef.doubleQuotString:
        if(codeStr[i-1]!=='\\' && c==='"' || c==='\r' || c==='\n') { state=stateDef.normal;}
        result.push(c);
      break;
      case stateDef.singleQuotString:
        if(codeStr[i-1]!=='\\' && c==="'" || c==='\r' || c==='\n') { state=stateDef.normal;}
        result.push(c);
      break;      
    }
  }
  return result.join('');
}
@xingrz

解决如何移除注释之前,先解决字符串对注释匹配的干扰

  util.parseDependencies = function(code) {
    // Parse these `requires`:
    //   var a = require('a');
    //   someMethod(require('b'));
    //   require('c');
    //   ...
    // Doesn't parse:
    //   someInstance.require(...);
    var ret = [], _strings = [], match;

    // extract all strings
    code = code.replace(/(["'])(.+?)\1/g, function(match, quote, str, index) {
      _strings.push(str);
      return match.replace(str, _strings.length - 1);
    });

    // remove comments
    code = code
      .replace(/\/\*[\s\S]*?\*\//mg, '') // block comments
      .replace(/\/\/.*$/mg, ''); // line comments

    // parse 'require()'
    REQUIRE_RE.lastIndex = 0;
    while ((match = REQUIRE_RE.exec(code))) {
      if (match[2]) {
        ret.push(_strings[match[2]])
      }
    }

    return util.unique(ret)
  }
@ethanzheng

@xingrz

-    code = code.replace(/(["'])(.+?)\1/g, function(match, quote, str, index) {
+    code = code.replace(/(['"])((:?\\\1|.)+?)\1/g, function(match, quote, str, index) {
@hax

我以前写过的那个highlighter是可以做的,而且性能很高。只是代码现在找不到。这里只有一个早期版本,但是删除注释应该足够了:http://hax.iteye.com/blog/181358 。Winter写的那个JS token正则应该也可以做,只是他考虑的情况更多,应该效率会低一些。

@feixun

第一步必须排除‘str’、"str"的干扰,就如我的微博评论说的,先遍历整个string,从""、''、/**/、//\n四种配对类型中先碰到那个符号,就打个标记,然后从标记处+1开始找配对的另一半符号,找到后打个标记,截取两个标记之间的字符窜,用占位符替换,剩下的递归处理,最后得到1个没有字符窜和注释干扰的string+占位符,以及一个token list,然后再遍历整个string,遇到占位符的时候,取1个token,检查token是否是注释,是则丢弃,不是则替换掉token符号,最后拼出来的就是去掉注释后的代码了。

@feixun

// 之前写过1个sql解析的,改一下就是用来处理代码的了
// 原谅我java用的还行,改成js会需要很多时间

package jsscan;

/**
 * 不允许直接创建实体类
 * 
 * @author feixun.dxf
 *
 */
public abstract class Token {

    /** 标记符的值 */
    protected String token;

    public String getToken() {
        return token;
    }

    public void setToken(String token) {
        this.token = token;
    }
}

package jsscan;

/**
 * 普通的code标记
 * 
 * @author feixun.dxf
 * 
 */
public class CodeToken extends Token {

    public CodeToken(String token) {
        this.token = token;
    }
}

package jsscan;

/**
 * 代表特殊标记(引号或者注释)的字符窜
 * 
 * @author feixun.dxf 2012.11.19
 * 
 */
public class LiteralToken extends Token {

    private LiteralDelimiter delimiter;

    public LiteralToken(String token, LiteralDelimiter delimiter) {
        this.token = token;
        this.delimiter = delimiter;
    }

    public LiteralDelimiter getDelimiter() {
        return delimiter;
    }

    public void setDelimiter(LiteralDelimiter delimiter) {
        this.delimiter = delimiter;
    }

}

package jsscan;

/**
 * 字符窜块分隔符枚举
 * 
 * @author feixun.dxf
 * 
 */
public enum LiteralDelimiter {

    SINGLEQUOTED("'", "'"), // 单引号
    DOUBLEQUOTED("\"", "\""), // 双引号
    CSTYLECOMMENT("/*", "*/"), // 块注释
    ROWCOMMENT("//", "\n"); // 行注释

    private final String start;
    private final String end;

    private LiteralDelimiter(final String start, final String end) {
        this.start = start;
        this.end = end;
    }

    public String getStart() {
        return start;
    }

    public String getEnd() {
        return end;
    }

    //

    /**
     * 是否是引号类型
     * 
     * @param delimiter
     * @return
     */
    public static boolean isQuoted(LiteralDelimiter delimiter) {
        if (delimiter == SINGLEQUOTED) {
            return true;
        } else if (delimiter == DOUBLEQUOTED) {
            return true;
        }
        return false;
    }

    /**
     * 是否是行注释类型
     * 
     * @param delimiter
     * @return
     */
    public static boolean isRowComment(LiteralDelimiter delimiter) {
        if (delimiter == ROWCOMMENT) {
            return true;
        }
        return false;
    }
}

package jsscan;

import java.util.List;
import java.util.StringTokenizer;
import java.util.Vector;

/**
 * 代码美化器
 * 
 * @author feixun.dxf
 *
 */
public class SourceBeautier {

    /** 占位符 */
    final static char PLACEHOLDER_TOKEN = '\001';

    /** 开始标记符 */
    final static char STX = '\002'; // 正文开始

    /** 结束标记符 */
    final static char ETX = '\003'; // 正文结束

    /** 初始的源码 */
    private String originalSource;

    /** 去除标记后的代码 */
    private String literalSource ;

    private List<LiteralToken> literalTokens = new Vector<LiteralToken>();
    private List<Token> allTokens = new Vector<Token>();

    public SourceBeautier(String Source) {
        this.originalSource = Source;
        init(); // 初始化
        tokenize();
    }

    /**
     * 取实例
     * 
     * @param text
     * @return
     */
    public static SourceBeautier getInstance(String Source) {
        return new SourceBeautier(Source);
    }

    /**
     * 属性初始化
     */
    private void init() {
        this.literalSource = processLiterals(originalSource, literalTokens);
    }

    /**
     * 处理text语句中特殊的标记符
     * 
     * @param source
     * @param tokens
     * @return
     */
    private static String processLiterals(final String source, List<LiteralToken> tokens) {
        StringBuilder text = new StringBuilder();
        for (int i = 0; i < source.length(); i++) {
            // 两次for循环,找出所有块字符窜
            for (final LiteralDelimiter delimiter : LiteralDelimiter.values()) {
                if (source.regionMatches(i, delimiter.getStart(), 0, delimiter.getStart().length())) { // 标记匹配
                    final int beforeStart = i;
                    final int afterStart = beforeStart + delimiter.getStart().length();
                    final int beforeEnd = source.indexOf(delimiter.getEnd(), afterStart); // 用indexOf,会不会遗漏特殊情况?
                    final int afterEnd = beforeEnd + delimiter.getEnd().length();
                    if (beforeEnd == -1) {
                        // 未配对,抛出异常,检查Source语句是否书写有误
                        throw new IllegalArgumentException("Literal delimiter pairing failed error.");
                    }
                    String escapedText = source.substring(beforeStart, afterEnd); // 包括了文字分隔符
                    tokens.add(new LiteralToken(escapedText, delimiter));

                    // 占位符
                    String holder = String.valueOf(PLACEHOLDER_TOKEN);
                    if (!LiteralDelimiter.isQuoted(delimiter)) {
                        if (i > 0 && source.charAt(i - 1) != ' ') {
                            holder = " " + holder;
                        }
                        if (afterEnd < (source.length() - 1) && source.charAt(afterEnd + 1) != ' ') {
                            holder = holder + " ";
                        }
                    }
                    text.append(holder);

                    // 递归剩余的source parts
                    return text.append(processLiterals(source.substring(afterEnd), tokens)).toString();
                }
            }
            text.append(source.charAt(i));
        }
        return text.toString();
    }

    private void tokenize() {
        int index = 0; // literal tokens index
        StringTokenizer tokenizer = new StringTokenizer(literalSource); // 去除空格和\t\n\r\f等5个字符
        while (tokenizer.hasMoreTokens()) {
            String token = tokenizer.nextToken();
            if (token.length() == 0) {
                continue;
            }
            token += ETX; // 增加行结束标记,方便字符窜处理
            // 处理占位符
            StringBuilder sb = new StringBuilder();
            char c = 0; // Null char
            for (int i = 0; i < token.length(); i++) {
                c = token.charAt(i);
                switch (c) {
                case PLACEHOLDER_TOKEN:
                    addToken(allTokens, sb);
                    addToken(allTokens, literalTokens.get(index));
                    index++; // next token
                    break;
                case ETX:
                    // 字符窜结束
                    addToken(allTokens, sb);
                    break;
                default:
                    sb.append(c);
                    break;
                }
            }
        }
    }

    /**
     * 将token追加到List,同时清空sb的元素
     * 
     * @param tokenList
     * @param sb
     */
    private static void addToken(List<Token> tokenList, StringBuilder sb) {
        if (sb.length() > 0) {
            tokenList.add(new CodeToken(sb.toString()));
            sb.setLength(0); // 清空
        }
    }

    /**
     * 将token追加到List
     * 
     * @param tokenList
     * @param token
     */
    private static void addToken(List<Token> tokenList, Token token) {
        tokenList.add(token);
    }

    /**
     * 得到美化的Source
     * 
     * @return
     */
    public String beautiful() {
        StringBuilder code = new StringBuilder();
        for (Token token : allTokens) {
            String fragment = token.getToken();
            if (token instanceof LiteralToken) {
                // 检查是否是注释
                if (fragment.indexOf("//") > 0 || fragment.indexOf("/*") > 0) {
                    continue;
                }
            }
            code.append(fragment);
        }
        return code.toString().trim();
    }

    /**
     * 静态工具方法,美化Source语句
     * 
     * @param source
     * @return 返回美化的Source语句
     */
    public static String beautiful(String source) {
        SourceBeautier beautier = new SourceBeautier(source);
        return beautier.beautiful();
    }
}

// 这里贴代码效果不行

@sofish

试试这个,48行,主要思路是利用 DOM:

var nocomment = function(str) {
  var div = document.createElement('div')
    , inline = /([^\\])\/\/[^\n]*(\n?)/g
    , reg = /\/[^\/]*\/g?m?i?[^\*]?/g
    , commentLikeReg = /\/\\\/\*[^\/]*\*\//g
    , string = new RegExp('(\'[^\']*\')|("[^"]*")','g')
    , identifier = 'SOFISH_S_METHOD_TO_REMOVE_JS_COMMENTS_'
    , matches = []
    , counter = 0
    , strReplace;

  strReplace = function(reg, str) {
    return str.replace(reg, function(match) {
      matches.push(match);
      return match.replace(match, identifier + (counter++));
    })
  }

  str = str.replace(reg, function(match) {
    var reg, type;
    try {
      reg = eval(match)
    } catch(e) {}
    type = Object.prototype.toString.call(reg);
    return type === '[object RegExp]' ? 
      (matches.push(match), match.replace(match, identifier + (counter++))) : 
      match;
  })

  str = strReplace(string, str);
  str = strReplace(commentLikeReg, str);

  str = str.replace(/\/\*([^@])/g, '<!-- $1').replace(/[^@](\*\/)/g, ' -->');
  str = str.replace(/(<!-- )[^>]*<!--/g, function(match, selected){
    return match.replace(selected, '*'); 
  });

  div.innerHTML = str;
  str = div.textContent;

  str = str.replace(inline, function(match, g1, g2){
    return match[0] + g2;
  });

  return str.replace(new RegExp(identifier + "(\\d+)", 'g'), function(match, selected) {
    return matches[selected];
  });
}
@xingrz

怎么越写越长了。。。。

@xingrz

另外,还有一种蛋疼的情况

someFun("this is not a require('xxx') !");

所以我觉得寻找依赖,不要仅仅去掉注释再匹配就完事了

依旧坚持这个思路:

  1. 先把字符串抽出来替换成唯一的占位
  2. 去掉注释(反正也没有干扰了)
  3. 匹配require,用占位符找回对应的模块名
@ziyunfei

貌似不难吧,没仔细看要求.写了一个.明天再好好改改

http://www.cnblogs.com/ziyunfei/archive/2012/12/18/2823692.html

几个条件都还都没看.性能什么的

@luolonghao

根据@hax的正则修改了一下,有些情况还是有问题。

  function removeComments(code) {
    return code.replace(/"(?:\\"|[^"])*"|'(?:\\'|[^'])*'|([^\\])([/][*][\S\s]*?(?:[*][/]|$)|[/][/].*)/g, function($0, $1) {
      return $1 || $0;
    });
  }
@lifesinger lifesinger reopened this Dec 19, 2012
@lifesinger
seajs member

@panasonic 的正则很神,待会我想想测试下,大赞。

@lifesinger
seajs member

@army8735 的是为 seajs 特殊定制版,也赞。

@lifesinger
seajs member

@hax hax 这个正则,不太符合需求,把所有字符串都干掉了,包括 require("xxx") 被变成了 require()

@lifesinger
seajs member

@feixun 感谢提供 Java 代码,这个不太符合要求,代码量太大。目前是想找一个比较轻巧的方案来实现,完整的方案已经有了,通过 UglifyJS 等来操作,可以很安全很完美的做到注释的去除,以及 require 的提取。

@lifesinger
seajs member

@sofish 思路挺有意思,还利用了 DOM,目前有一个测试用例没通过:

var r = /\/*require('mod')*/;
@lifesinger
seajs member

@ziyunfei 非常强大,目前看到的准确率最高的,待会我再详细测下。

@lifesinger
seajs member

@luolonghao 除了极端情况,测试用例全部通过,正则居然可以搞定,强的。

@pierrewrs

我看到的packer工具的思路是一次正则替换完成的, replacer 方法有些复杂,然后关键是用到的 RegGrp 这个东东好强大,需要静下心来好好读 https://code.google.com/p/base2/source/browse/trunk/src/apps/packer/php/base2/RegGrp.php?r=231 。 多步 replace 方案总觉得会有漏洞的〜

@ethanzheng
@ziyunfei

刚才又想了一下,发现我昨天写的代码问题太多了.主要是除号和ASI的问题.
我现在认为:不进行完整的语法分析,是不可能实现这个需求的.不光正则不可能,就连简单的语法分析也不行.必须是完整的JavaScript parser才能做到正确的删除注释这一需求.

packer的移除注释也有问题,YUI Compressor的移除注释也有问题,Firebug中注释的代码着色也有问题,只有Esprima是完美的http://esprima.org/demo/parse.html?code=%2F%20%2F%20%2F%20%2F%20%2F%2F%2F

@ziyunfei

另外,楼主说Firefox中函数执行toString后的结果和其他浏览器不一样,更新你的FF,然后看我的这篇文章就明白了
http://www.cnblogs.com/ziyunfei/archive/2012/12/04/2799603.html

@army8735
seajs member

举个例子说说yui的错误地方

@army8735
seajs member
@ziyunfei

@army873
就是测试页面http://seajs.org/tests/research/remove-comments-safely/test.html
里的最后一个极端情况啊. var a=/ // /b\//g;
YUI Compressor是基于完整的js引擎(Rhino)的,按理说表现也应该要完美的.
另外,测试页面现在的这个方法在源码里有除号的时候会失败
var a = 8 / 2; //require("module A")

@army8735
seajs member

@ziyunfei 清桑,这个地方我也犯过错误。
那另外一个问题,为何要引入完全的语法分析?据我思索只要简易的词法分析器即可,用不到语法分析器。

@ziyunfei

@army8735
你说的对.这个我不懂了.

@luolonghao

改进后的正则,除了最后一个极端情况都可以。

function removeComments(code) {
    return code.replace(/"(?:\\"|[^"])*"|'(?:\\'|[^'])*'|[/][*][\S\s]*?(?:[*][/]|$)|[/](?:\\[/]|[^/])+[/]|[/][/].*/g, function(match) {
        return /^([/][*]|[/][/])/.test(match) ? '' : match;
    });
}
@luolonghao

悲剧,var a = 8 / 2; //require("module A") 这个情况还是不行,看来只能引入简易词法分析器。

@tonny-zhang

@luolonghao @lifesinger 看下http://jsperf.com/remove-comments/3 里的removeComment_tonny ,@luolonghao的情况也可以实现

@yessky

这个应该能通过你的测试页面吧
http://test.veryos.com/research/remove-comments/

不知道移除mg 或者二者之一正则能否正常工作,移除应该效率会有一定提升吧

function k_remove_comments(code) {
    // multi-line comments
    code = code.replace(/^\s*\/\*[\s\S]*?\*\/\s*(\n|\r|\f)?/mg, '$1');

    // single-line comments starts with '//'
    code = code.replace(/^\s*\/\/[^\r\n\f]*?(\r|\n|\f|$)/mg, '');

    // single-line comments not starts with '//'
    code = code.replace(/(;|,|}|])\s*\/\/[^\r\n\f]*?(\r|\n|\f|$)/mg, '$1$2');

    return code;
  }
@lifesinger
seajs member

回归到原始需求,是提取依赖信息。我做了一个测试页面:

http://seajs.org/tests/research/parse-dependencies/test.html

汇集了 9 种方案,其中能满足 seajs 提取依赖需求的只有两种,我后来想到的一种方案,以及 Army 提供的。

性能测试:

http://jsperf.com/parse-dependencies

我的方案有点取巧,在原来的基础上,让 ; 变成 ;\n 来规避 require 在字符串 // 之后的问题。
Army 的方案目前性能有点不妥当。

大家可以再看看。

@lifesinger
seajs member

目前顶楼的问题已解决,成本最低的方案。同时推送到 RequireJS 社区等待 merge 中。

@lifesinger
seajs member

奖金继续有效,要求改成:

  1. 解决 seajs 依赖提取的实际需求,即可通过 parse-dependencies/test.html 的测试。
  2. 性能不比 seajs 现有方案的差,可参考 http://jsperf.com/parse-dependencies
  3. 代码量不比 seajs 现有方案的多(替换掉 src/util-deps.js 文件,压缩后代码不会变多)。

感谢参与。

@liuda101

留名先

@yessky

我还以为只是移除注释呢。。。

@pierrewrs

“在原来的基础上,让 ; 变成 ;\n 来规避 require 在字符串 // 之后的问题。” 这个要是有代码这样 "//abc; efg;require()" 是不是就躺枪了啊

@army8735
seajs member

目前基于状态机的性能还需要提升%多少?

@yessky

替换掉;不能解决根本问题。


var o = {
require: function() {
},
f:require('f')
}; // case1;require('whosyourdaddy');

@luolonghao

优化了一下,加了parseDependenciesByLuolonghao2,在Chrome上比parseDependenciesByLifesinger2快一些,http://jsperf.com/parse-dependencies2

@lifesinger
seajs member

@army8735 状态机那个,主要问题还是代码太多,集成进去,SeaJS 要增大将近 1k,不划算。这个功能是 seajs 里很小的一个功能点。性能目前也有不妥,试了下 500 个 800 行的文件,在我电脑上需要耗费近 5s 左右时间。

@army8735
seajs member

@lifesinger gzip后无需担心,性能改了下,8000行用了160ms,第2个框。第3个框在ff下出奇得慢,其它浏览器略好于第2个框。40w行的代码出现在浏览器端的情况……

@lifesinger
seajs member

@luolonghao 仔细看了下这个正则,赞这个思路。我稍微调整了下,可以通过所有测试用例了。

http://seajs.org/tests/research/parse-dependencies/test.html?v2

性能也很不错:

http://jsperf.com/parse-dependencies/3

另外赞 army 的状态机,在测试用例不断变更的情况下,army 的代码一路 PASS

考虑性能和代码量,我暂时在 SeaJS 里还是采用了下面的代码:

https://github.com/seajs/seajs/blob/master/src/util-deps.js

等到这个月底,如果没有更优秀的方案出现,就给 @army8735@luolonghao 奖金各 50 美刀吧,呵呵。

@wintercn

五个case 都能过头给你们......

var a = 'xxxx \
// xxxx\ require("aaa") \
'

var x =/ x /* 333
require("aaa")
/*
^_^
*/

var x =2/ x /* 333
require("aaa")
/*
^_^
*/

if(a+b)/ x /* 333
require("aaa")
/*
^_^
*/

(a+b)/ x /* 333
require("aaa")
/*
^_^
*/
@ziyunfei

^_^ 我写的过了

@wintercn

嗯 看来至少我已经完爆了github的语法高亮了 哈哈哈

@saighost

汗。。。@wintercn 第一个最后的的符合有问题啊?

@army8735
seajs member

@wintercn 居然让我发现一个隐蔽的bug,好久没动了……lexer来战表示ok:http://jssc.googlecode.com/svn/trunk/jssc5/bin/index.html

@luolonghao

@army8735 页面语言居然有韩文,不过写错了,呵呵

@hax

计子终于出现了,哈哈。感谢@luolonghao ,奖金记得分我点哈。

@lifesinger
seajs member

我决定忽略 @wintercn 提供的 rare cases
就这样,决定了,哈哈

@stevezheng
@luolonghao

@lifesinger @hax 好的,一人25美刀,哈哈。

@yessky

var xxxx = 'require("showmethemoney")';

也被解析成了依赖?

@army8735
seajs member

@luolonghao 복사?

@luolonghao

@army8735 嗯,还有把韩国语写成韩国人了。

韩国语 -> 한국어
复制 -> 복사하기

@lifesinger
seajs member

@yessky 正则版本的确有这个问题,基于正则版本,哪位挑战下下面这个 case:

var xxxx = 'require("showmethemoney")';
@luolonghao

@lifesinger REQUIRE_RE好像也可以用NOISE_RE的思路,改成

var REQUIRE_RE = "(?:\\"|[^"])*"|'(?:\\'|[^'])*'|(?:^|[^.$])\brequire\s*\(\s*(["'])(.+?)\1\s*\)

然后在while里跳过字符串。

@lifesinger
seajs member

@luolonghao 嗯,的确可以,发现可以进一步把 NOISE_RE 也干掉,直接:

;(function(util) {
  var REQUIRE_RE = /"(?:\\"|[^"])*"|'(?:\\'|[^'])*'|\/\*[\S\s]*?\*\/|\/(?:\\\/|[^/])+\/|\/\/.*|(?:^|[^.$])\brequire\s*\(\s*(["'])(.+?)\1\s*\)/g

  util.parseDependencies = function(code) {
    var ret = [], m
    REQUIRE_RE.lastIndex = 0

    while ((m = REQUIRE_RE.exec(code))) {
      if (m[2]) ret.push(m[2])
    }

    return util.unique(ret)
  }

})(seajs._util)

神一样的代码呀,哈哈,无论性能还是准确率,都比原来的好,赞。

@ethanzheng
$(element).
    width().
    height().
    require("./AAA")

这个似乎不是极端用例, AAA模块将被认为是个依赖。

@ipirlo

昨天看了下测试页面,发现只@army8735 的能正确处理字符串,
其他的只对引号做特殊处理,其实反斜线也是特殊的。
比如试试这个
var a = "They stretch'd in never-ending line.\\";
或者
var bs ='\\\\';

@army8735
seajs member

/require("a")/
/\/require("a")/
/[/]require("a")/

@luolonghao

@army8735 前两个没问题,正则表达式部分的正则这样写可以过第三个情况,不过正则有点长。

/\/(?:\\\/|\[(?:\\\]|[^\]])*\/(?:\\\]|[^\]])*\]|[^/])+\//
;(function(util) {
  var REQUIRE_RE = /"(?:\\"|[^"])*"|'(?:\\'|[^'])*'|\/\*[\S\s]*?\*\/|\/(?:\\\/|\[(?:\\\]|[^\]])*\/(?:\\\]|[^\]])*\]|[^/])+\/|\/\/.*|(?:^|[^.$])\brequire\s*\(\s*(["'])(.+?)\1\s*\)/g

  util.parseDependencies = function(code) {
    var ret = [], m
    REQUIRE_RE.lastIndex = 0

    while ((m = REQUIRE_RE.exec(code))) {
      if (m[2]) ret.push(m[2])
    }

    return util.unique(ret)
  }

})(seajs._util)

@ipirlo 反斜线情况已经考虑了。

@ethanzheng 那种情况确实有问题。

@army8735
seajs member

/[\/]require("a")/
/\[/]require("a")/
/[[/require("a")/
/[]require("a")]/
/\\/require("a")/
/[require("a")]/

@army8735
seajs member

上面有人也给了正确的解释:上下文无关文法中是不能用正则匹配出全部情况的,只能接近大部分情况。
随着正则的复杂度提升,它的阅读和维护也越来越困难。

@dotnil

跳出來支持一下 @army8735 的觀點

@lepture
seajs member

我想说一下:谁把代码写得这么蛋疼?拉出来示众一下。

作为开发时的 require 解析,真的有必要追求完美否?既然写出了如此蛋疼的代码,就让他蛋疼去吧。

珍爱生命,远离垃圾代码。

@dotnil

注释、字符串、正则里有 require 的情况不能完全排除吧,既然支持了解析特性,就在能力范围内做到尽善尽美,挺好的啊;而且,已经忽略掉了 @wintercn 提出来的几个生僻用例了哇。

@xingrz

同意 @army8735 的观点。

我觉得在开发环境下只需要清晰、优雅,能匹配大部分情况就行了。

打包的时候再用精准、或许更复杂的方式去处理。

@luolonghao

@army8735 你写的这几个正则都语法错误了呀。

/[\/]require("a")/
/\[/]require("a")/
/[[/require("a")/
/[]require("a")]/
/\\/require("a")/
/[require("a")]/

其实上面正则最大的问题是无法区分除号,比如这种情况还是很常见的。

var a = 1 / 2; //require("a")
@army8735
seajs member

不是我写错了,估计是提交过程中\被当成转义符和后面的字符合并了

@luolonghao

@lifesinger 现在的正则处理这样的代码有严重问题,不会加载require("a")。

var a = c / b

require("a")

//require("b")

加\r\n后应该没问题。

var REQUIRE_RE = /"(?:\\"|[^"])*"|'(?:\\'|[^'])*'|\/\*[\S\s]*?\*\/|\/(?:\\\/|\[(?:\\\]|[^\]])*\/(?:\\\]|[^\]])*\]|[^/\r\n])+\/|\/\/.*|(?:^|[^.$])\brequire\s*\(\s*(["'])(.+?)\1\s*\)/g

不过这样的代码还是有问题,会加载require("c"),感觉考虑除号的话正则基本无解。

var a = c / b; //require("c")
@ipirlo

这个处理不极端的情况应该没有问题。对REQUIRE_RE稍做修改,可以处理 @ethanzheng 提出的问题。
不过如果直接使用一个嵌套定义的require函数,就没有办法了。

function skip_collect(src) {
    var quote = /(?:'(?:[^\\']|\\[\s\S])*')|(?:"(?:[^\\"]|\\[\s\S])*")/;
    var comment = /(?:\/\/.*)|(?:\/\*[\s\S]*?\*\/)/;
    //如果斜线之前出现这些,就可以判断为正则。注释掉的这个已经很繁冗,但仍然不全面。。
    //通常情况没必要考虑这么多,性能上也是个问题,所以只留下我想到的正常情况会需要的。
    //var regex_prefix=/(?:^|\b(?:typeof|return|delete|void)|[-;:({},?+=]|\|\||&&)\s*/;
    var regex_prefix = /(?:\b(?:return)|[[;:(,?=]|\|\||&&)\s*/;
    var regex4regex = /\/(?!\*)(?:[^[/\\]|\\.|(?:\[\^?(?:[^\]\\]|\\.)*\]))+\/[img]{0,3}/;
    var skip = RegExp(quote.source + '|(?:' + regex_prefix.source + regex4regex.source + ')|' + comment.source, 'g');
    //就是下面这个东西,这样看起来不太伤眼
    //var skip=/(?:'(?:[^\\']|\\[\s\S])*')|(?:"(?:[^\\"]|\\[\s\S])*")|(?:(?:\b(?:return)|[[;:(,?=]|\|\||&&)\s*\/(?!\*)(?:[^[\/\\]|\\.|(?:\[\^?(?:[^\]\\]|\\.)*\]))+\/[img]{0,3})|(?:\/\/.*)|(?:\/\*[\s\S]*?\*\/)/g;
    var result = [];
    //处理dot之后换行的问题
    var REQUIRE_RE = /(?:^|[^.\s$])\s*\brequire\s*\(\s*(["'])(.+?)\1\s*\)/g;
    for (var start = 0, skip_match, req_match;
      //require的参数是字符串,正是skip要匹配的一种情况
      (skip_match = skip.exec(src)) != null; start = skip_match.index + skip_match[0].length) {
        if (skip_match[0].charAt(0) != '"' && skip_match[0].charAt(0) != "'") continue;
        REQUIRE_RE.lastIndex = start;
        while ((req_match = REQUIRE_RE.exec(src)) != null && req_match.index < skip_match.index)
           result.push(req_match[2]);
    }
    return result;
}

测试集里这两个正则没有识别出来。

//这个居然符合语法?看不明白,求解释
  var rare=/ // /b\//g;

//只用正则搞不定计子这样的。。。
  if(a+b)/ x /* 333
  require("rare5")
  /*
  ^_^
  */
@ipirlo

由于在regex里可以出现非转义的//, /*, ', "

/[//]no 'comment[*/]/

所以在移除注释时必须解决这个问题(至少部分解决,不考虑极端情况)。
要正确的找出regex,真正困难的是分清 '/' 什么时候是除号。
我们需要看看斜线之前的内容才能判断:
如果是number或identifier,那肯定是除号;
如果是运算符或( [ : ;或return等,则是正则;
如果是),就难说了,因为只用正则无法找到与之配对的( ,也就无法确定括号内容是不是条件或循环语句的判断部分。
不过一般应该不会有这么无聊:

while(a<b)
    /[/*]/g.exec('...');

有兴趣的可以看看这个[讨论] 挑战:万能的斜线! 判断js代码里面 / 是正则、除号、注释?

@ipirlo

@luolonghao

/\/(?:\\\/|\[(?:\\\]|[^\]])*\/(?:\\\]|[^\]])*\]|[^/])+\//

这个正则有问题,有可匹配过头,比如:

function hasBS(s){
   return /\\/.test(s)//unnecessary comments considered harmful
}

==>

/\\/.test(s)/

( a=/[/\\](\d+)/.exec(s) ) && a[1]/4>0

==>

/[/\\](d+)/.exec(s))&&a[1]/

和字符串的处理一样,问题还是出在反斜线

\]   \'  \/  ...

中的反斜线有可能是被转义的,如果我们把它误认为是转义符,对其后字符的解释就可能出错
在正则中,如果不对反斜线专门处理就会出现这样的问题。

@army8735
seajs member

@ipirlo var rare=/ // /b\//g;的意思是一个正则除以一个正则
即使不考虑正则情况,字符串中的转义符和换行就够呛
除号难题存在于支持perl风格正则的文法当中,我也没接触过权威的文献,仅凭几年的积累而已:
http://jssc.googlecode.com/svn/trunk/jssc5/src/lexer/LanguageLexer.as

@ipirlo

@army8735 加减乘都测试过,就是没想到除。。
除号的难点在括号,单靠正则处理不了嵌套的括号表达式。
好在if/for/while(...)之后紧接一个正则常量不是正常用法。

字符串完全没问题,只要正确处理\
\\ \" \" \n \t等等,包括行末加\使字符串延续到下一行(相当于\+<newline>
都是反斜线使其后的字符意义转变,所以只要在正则里把\和其后的字符视为一个整体就行:

[^\\']|\\.

把字符串中可能出现的字符分为两类:
1. 转义组合:\和其后的字符。
\'只是其中之一,而测试页面里的大部分方案把它当作唯一的特殊情况,问题就出在这。
2. 除\和序列结束符之外的所有字符。这些字符可以随意出现。
对于所有两端由某些符号确定、其中可能出现转义的序列都可以用这种方法处理,比如正则中的[:char class:]

《精通正则表达式》 里讲得很清楚。

@lifesinger
seajs member

综合大家的意见,更新了测试页面:

http://seajs.org/tests/research/parse-dependencies/test.html?1223

目前 SeaJS 采用了:

/**
 * The parser for dependencies
 * Ref: tests/research/parse-dependencies/test.html
 */
;(function(util) {

  var REQUIRE_RE = /"(?:\\"|[^"])*"|'(?:\\'|[^'])*'|\/\*[\S\s]*?\*\/|\/(?:\\\/|[^/\r\n])+\/(?=[^\/])|\/\/.*|\.\s*require|(?:^|[^$])\brequire\s*\(\s*(["'])(.+?)\1\s*\)/g
  var SLASH_RE = /\\\\/g

  util.parseDependencies = function(code) {
    var ret = [], m
    REQUIRE_RE.lastIndex = 0
    code = code.replace(SLASH_RE, '')

    while ((m = REQUIRE_RE.exec(code))) {
      if (m[2]) ret.push(m[2])
    }

    return util.unique(ret)
  }

})(seajs._util)

目前这段代码可以通过非 winter 的所有测试案例。

保持一个宗旨:

  1. 尽量覆盖各种情况,但不求完全覆盖,比如 winter 用除号做的那些 rare case
  2. 尽量用最短小且性能好的方式来达成

之所以有这个选择,是因为 parse dependencies 在 SeaJS 里只是一个很小的功能点,并且上线后不依赖此功能(上线前会用更可靠的 spm 通过 UglifyJS 操作 AST 来获取依赖信息)。

@army8735
seajs member

@ipirlo 我说的是Ecmascript的权威文法我没看过……if/for/while那个也很容易判断……
我推测随着v8的发展,状态机的速度会逐渐超过正则,因为正则本身就是需要转化为自动机解析的。as里正则的消耗很早就远大于语言本身了,浏览器里没辙,js解释器太慢了,比不过正则的执行速度,不知目前chrome中两者相差多少……

@lifesinger
seajs member

@army8735 目前状态机和正则相比,对于解决依赖提取问题,还是差得非常明显,对比: http://jsperf.com/parse-dependencies/3

@tonny-zhang

/
"(?:\"|[^"])"| #双引号里内容
'(?:\'|[^'])
'| #单引号里内容
\/*[\S\s]?\\/| #多行注释符里内容
\/(?:\\/|[^/\r\n])+\/(?=[^\/])| #处理正则表达式
\/\/.| #处理单行注释
.\s
require| #处理a.require的特殊情况
(?:^|[^$])\brequire\s(\s(["'])(.+?)\1\s*) #提取require
/g
上面是关于 @lifesinger 用到正则的说明,求拍砖

@lifesinger
seajs member

@tonny-zhang 完全正确,呵呵

还有一个补充下

var SLASH_RE = /\\\\/g
code = code.replace(SLASH_RE, '')  // 处理以 \\ 结束的字符串

@tonny-zhang
var SLASH_RE = /\\\\/g
code = code.replace(SLASH_RE, '')  // 处理以 \\ 结束的字符串

@lifesinger 太高明了,完全排除\\对字符串的干扰

@lifesinger
seajs member

@army8735 @hax @luolonghao 三位把支付宝帐号给我(lifesinger@gmail.com
按期发奖金啦

@army8735
seajs member
@afc163
seajs member
This was referenced Sep 12, 2014
@afc163 afc163 changed the title from 解析依赖时,移除代码注释产生的问题 to 解析依赖时,移除代码注释产生的问题(内有提取依赖的优化活动) Sep 12, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment