From 5dd8e797f7babbc971874f0e2c80073f7077d9db Mon Sep 17 00:00:00 2001 From: David Majda Date: Wed, 5 Oct 2016 11:09:03 +0200 Subject: [PATCH] Code style: Do not lint lib/parser.js The idea behind linting lib/parser.js was that it would improve quality of code generated by PEG.js in general. However, there is a couple of problems with it: 1. Code in lib/parser.js is ES5 while the rest of the code is ES2015. This would mean a separate ESLint configuration and a separate set of code style rules just for lib/parser.js once code style checks are added. 2. Code in lib/parser.js is generated. This means that even today it violates checks like "no-unused-var", which have to be disabled. This would get worse once code style checks are added, again requiring a separate ESLint configuration just for lib/parser.js. 3. Linting lib/parser.js checks only small portion of possible code generator output. For example, code generated when optimizing for size or when tracing is not checked at all. Thus, linting lib/parser.js gives a false sense of security. Because of these problems I decided not to lint lib/parser.js at all and rely on ad-hoc linting of parser files produced by PEG.js with ignoring false-positives. I consider this more of a pragmatic cost vs. benefits decision than a principial one. Part of #407. --- Makefile | 22 ++++------------------ lib/parser.js | 2 -- 2 files changed, 4 insertions(+), 20 deletions(-) diff --git a/Makefile b/Makefile index 16029da..56cdd35 100644 --- a/Makefile +++ b/Makefile @@ -17,9 +17,8 @@ NODE_MODULES_BIN_DIR = $(NODE_MODULES_DIR)/.bin MAIN_FILE = $(LIB_DIR)/peg.js -PARSER_SRC_FILE = $(SRC_DIR)/parser.pegjs -PARSER_OUT_FILE = $(LIB_DIR)/parser.js -PARSER_OUT_FILE_NEW = $(LIB_DIR)/parser.js.new +PARSER_SRC_FILE = $(SRC_DIR)/parser.pegjs +PARSER_OUT_FILE = $(LIB_DIR)/parser.js BROWSER_FILE_DEV = $(BROWSER_DIR)/peg.js BROWSER_FILE_MIN = $(BROWSER_DIR)/peg.min.js @@ -45,20 +44,7 @@ all: browser # Generate the grammar parser parser: - # We need to prepend ESLint header to the generated parser file because we - # don't want the various unused variables there to get reported. This is a bit - # tricky because the file is used when generating its own new version, which - # means we can't start writing the header there until we call $(PEGJS). - - $(PEGJS) -o $(PARSER_OUT_FILE_NEW) $(PARSER_SRC_FILE) - - rm -f $(PARSER_OUT_FILE) - - echo '/* eslint-disable no-unused-vars */' >> $(PARSER_OUT_FILE) - echo >> $(PARSER_OUT_FILE) - cat $(PARSER_OUT_FILE_NEW) >> $(PARSER_OUT_FILE) - - rm $(PARSER_OUT_FILE_NEW) + $(PEGJS) -o $(PARSER_OUT_FILE) $(PARSER_SRC_FILE) # Build the browser version of the library browser: @@ -101,7 +87,7 @@ benchmark: # Run ESLint on the source lint: $(ESLINT) \ - `find $(LIB_DIR) -name '*.js'` \ + `find $(LIB_DIR) -name '*.js' -and -not -path '$(PARSER_OUT_FILE)'` \ `find $(SPEC_DIR) -name '*.js' -and -not -path '$(SPEC_DIR)/vendor/*'` \ $(SPEC_SERVER_FILE) \ $(BENCHMARK_DIR)/*.js \ diff --git a/lib/parser.js b/lib/parser.js index 738c1ae..cb8c5a5 100644 --- a/lib/parser.js +++ b/lib/parser.js @@ -1,5 +1,3 @@ -/* eslint-disable no-unused-vars */ - // Generated by PEG.js 0.10.0. // // http://pegjs.org/