Skip to content

Commit

Permalink
Bug fix for JS semicolon detection (codecombat#5101)
Browse files Browse the repository at this point in the history
  • Loading branch information
AndrewJakubowicz authored Jan 11, 2019
1 parent bb8eeca commit 3302c3c
Show file tree
Hide file tree
Showing 12 changed files with 42 additions and 38 deletions.
2 changes: 0 additions & 2 deletions app/lib/aether/languages/java.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,9 @@ parserHolder = {}
module.exports = class Java extends Language
name: 'Java'
id: 'java'
parserID: 'cashew'

constructor: ->
super arguments...
parserHolder.cashew ?= self?.aetherCashew ? require 'cashew-js'
@runtimeGlobals = ___JavaRuntime: parserHolder.cashew.___JavaRuntime, _Object: parserHolder.cashew._Object, Integer: parserHolder.cashew.Integer, Double: parserHolder.cashew.Double, _NotInitialized: parserHolder.cashew._NotInitialized, _ArrayList: parserHolder.cashew._ArrayList


Expand Down
35 changes: 18 additions & 17 deletions app/lib/aether/languages/javascript.coffee
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
_ = window?._ ? self?._ ? global?._ ? require 'lodash' # rely on lodash existing, since it busts CodeCombat to browserify it--TODO

# jshintHolder = {}
jshintHolder = {}
escodegen = require 'escodegen'

Language = require './language'
Expand All @@ -16,7 +16,8 @@ module.exports = class JavaScript extends Language

constructor: ->
super arguments...
# jshintHolder.jshint ?= (self?.aetherJSHint ? require('jshint')).JSHINT
{ JSHINT } = require('jshint')
jshintHolder.jshint ?= JSHINT

# Return true if we can very quickly identify a syntax error.
obviouslyCannotTranspile: (rawCode) ->
Expand Down Expand Up @@ -58,21 +59,21 @@ module.exports = class JavaScript extends Language
# return lintProblems unless jshintHolder.jshint
wrappedCode = @wrap rawCode, aether

# # Run it through JSHint first, because that doesn't rely on Esprima
# # See also how ACE does it: https://github.com/ajaxorg/ace/blob/master/lib/ace/mode/javascript_worker.js
# # TODO: make JSHint stop providing these globals somehow; the below doesn't work
# jshintOptions = browser: false, couch: false, devel: false, dojo: false, jquery: false, mootools: false, node: false, nonstandard: false, phantom: false, prototypejs: false, rhino: false, worker: false, wsh: false, yui: false
# jshintGlobals = _.zipObject jshintGlobals, (false for g in aether.allGlobals) # JSHint expects {key: writable} globals
# # Doesn't work; can't find a way to skip warnings from JSHint programmatic options instead of in code comments.
# #for problemID, problem of @originalOptions.problems when problem.level is 'ignore' and /jshint/.test problemID
# # console.log 'gotta ignore', problem, '-' + problemID.replace('jshint_', '')
# # jshintOptions['-' + problemID.replace('jshint_', '')] = true
# try
# jshintSuccess = jshintHolder.jshint(wrappedCode, jshintOptions, jshintGlobals)
# catch e
# console.warn "JSHint died with error", e #, "on code\n", wrappedCode
# for error in jshintHolder.jshint.errors
# lintProblems.push aether.createUserCodeProblem type: 'transpile', reporter: 'jshint', error: error, code: wrappedCode, codePrefix: @wrappedCodePrefix
# Run it through JSHint first, because that doesn't rely on Esprima
# See also how ACE does it: https://github.com/ajaxorg/ace/blob/master/lib/ace/mode/javascript_worker.js
# TODO: make JSHint stop providing these globals somehow; the below doesn't work
jshintOptions = browser: false, couch: false, devel: false, dojo: false, jquery: false, mootools: false, node: false, nonstandard: false, phantom: false, prototypejs: false, rhino: false, worker: false, wsh: false, yui: false
jshintGlobals = _.zipObject jshintGlobals, (false for g in aether.allGlobals) # JSHint expects {key: writable} globals
# Doesn't work; can't find a way to skip warnings from JSHint programmatic options instead of in code comments.
#for problemID, problem of @originalOptions.problems when problem.level is 'ignore' and /jshint/.test problemID
# console.log 'gotta ignore', problem, '-' + problemID.replace('jshint_', '')
# jshintOptions['-' + problemID.replace('jshint_', '')] = true
try
jshintSuccess = jshintHolder.jshint(wrappedCode, jshintOptions, jshintGlobals)
catch e
console.warn "JSHint died with error", e #, "on code\n", wrappedCode
for error in jshintHolder.jshint.errors
lintProblems.push aether.createUserCodeProblem type: 'transpile', reporter: 'jshint', error: error, code: wrappedCode, codePrefix: @wrappedCodePrefix

# Check for stray semi-colon on 1st line of if statement
# E.g. "if (parsely);"
Expand Down
11 changes: 3 additions & 8 deletions app/views/editor/level/LevelEditView.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,10 @@ loadAetherLanguage = require("lib/loadAetherLanguage");
require 'vendor/scripts/coffeescript' # this is tenuous, since the LevelSession and LevelComponent models are what compile the code
require 'lib/setupTreema'

# Make sure that all of our Aethers are loaded, so that if we try to preview the level, it will work.
# require 'bower_components/aether/build/javascript'
# require 'bower_components/aether/build/python'
# require 'bower_components/aether/build/coffeescript'
# require 'bower_components/aether/build/lua'
# require 'bower_components/aether/build/java'
# require 'bower_components/aether/build/html'
# Make sure that all of our languages are loaded, so that if we try to preview the level, it will work.
require 'bower_components/aether/build/html'
Promise.all(
["javascript", "python", "coffeescript", "lua", "java"].map(
["javascript", "python", "coffeescript", "lua"].map(
loadAetherLanguage
)
)
Expand Down
7 changes: 1 addition & 6 deletions app/views/ladder/SimulateTabView.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,8 @@ module.exports = class SimulateTabView extends CocoView
@simulatorsLeaderboardData = new SimulatorsLeaderboardData(me)
@simulatorsLeaderboardDataRes = @supermodel.addModelResource(@simulatorsLeaderboardData, 'top_simulators', {cache: false})
@simulatorsLeaderboardDataRes.load()
# require 'bower_components/aether/build/javascript'
# require 'bower_components/aether/build/python'
# require 'bower_components/aether/build/coffeescript'
# require 'bower_components/aether/build/lua'
# require 'bower_components/aether/build/java'
Promise.all(
["javascript", "python", "coffeescript", "lua", "java"].map(
["javascript", "python", "coffeescript", "lua"].map(
loadAetherLanguage
)
)
Expand Down
5 changes: 2 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@
"body-parser": "^1.15.2",
"bootstrap": "^3.3.7",
"bootstrap-sass": "3.3.7",
"cashew-js": "^0.4.3",
"chalk": "^1.1.3",
"cheerio": "^0.22.0",
"chunk-splitting-plugin": "^2.4.0",
Expand All @@ -96,15 +95,15 @@
"geoip-lite": "^1.1.6",
"graceful-fs": "~2.0.1",
"gridfs-stream": "~1.1.1",
"htmlparser2": "^3.9.1",
"html-webpack-plugin": "^2.30.1",
"htmlparser2": "^3.9.1",
"intercom-client": "^2.8.8",
"jade": "^1.11.0",
"jade-loader": "codecombat/pug-loader#fix-errors",
"jquery": "~2.1.0",
"jquery-mousewheel": "~3.1.9",
"jquery.browser": "~0.0.6",
"jshint": "^2.9.4",
"jshint": "~2.3.0",
"json-loader": "^0.5.4",
"jsondiffpatch": "^0.2.3",
"lodash": "^2.4.2",
Expand Down
File renamed without changes.
File renamed without changes.
File renamed without changes.
18 changes: 18 additions & 0 deletions spec/aether/javascript_spec.coffee
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
Aether = require '../aether'

describe "Linting Test Suite", ->
describe "Default linting", ->
aether = new Aether()
it "Should warn about missing semicolons", ->
code = "var bandersnatch = 'frumious'"
warnings = aether.lint(code).warnings
expect(warnings.length).toEqual 1
expect(warnings?[0].message).toEqual 'Missing semicolon.'

describe "Custom lint levels", ->
it "Should allow ignoring of warnings", ->
options = problems: {jshint_W033: {level: 'ignore'}}
aether = new Aether options
code = "var bandersnatch = 'frumious'"
warnings = aether.lint(code).warnings
expect(warnings.length).toEqual 0
File renamed without changes.
1 change: 0 additions & 1 deletion test/app/aether.js

This file was deleted.

1 change: 0 additions & 1 deletion webpack.base.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ module.exports = (env) => {
]),
lodash: 'lodash', // For worker_world
aether: './bower_components/aether/build/aether.js', // For worker_world
// aether: './public/javascripts/aether.js',
// esper: './bower_components/esper.js/esper.js',
// vendor: './app/vendor.js'
},
Expand Down

0 comments on commit 3302c3c

Please sign in to comment.