Add a new "missing_require_severity" config option (#1773)

This option will command how the builder should behave when a required file is missing. Previously the behavior was undefined and partly controlled by dynamic_require_severity.
opal · Mar 6, 2018 · 9374f98 · 9374f98
1 parent 73c1268
commit 9374f98
Show file tree

Hide file tree

Showing 12 changed files with 125 additions and 32 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -68,6 +68,7 @@ Whitespace conventions:
 - Added `ndigits` option support to `Number#floor`, `Number#ceil`, `Number#truncate`. (#1757)
 - Added `key` and `receiver` attributes to the `KeyError`. (#1757)
 - Extended `Struct.new` to support `keyword_init` option. (#1757)
+- Added a new `Opal::Config.missing_require_severity` option and relative `--missing-require` CLI flag. This option will command how the builder will behave when a required file is missing. Previously the behavior was undefined and partly controlled by `dynamic_require_severity`. Not to be confused with the runtime config option `Opal.config.missing_require_severity;` which controls the runtime behavior.
 
 
 ### Changed
@@ -96,6 +97,7 @@ Whitespace conventions:
 - Allow passing number of months to `Date#next_month` and `Date#prev_month`. (#1757)
 - Fixed `pattern` argument handling for `Enumerable#grep` and `Enumerable#grep_v`. (#1757)
 - Raise `ArgumentError` instead of `TypeError` from `Numeric#step` when step is not a number. (#1757)
+- At run-time `LoadError` wasn't being raised even with `Opal.config.missing_require_severity;` set to `'error'`.
 
 
 ## [0.11.0] - 2017-12-08

diff --git a/lib/opal/builder.rb b/lib/opal/builder.rb
@@ -63,12 +63,13 @@ def initialize(options = nil)
         public_send("#{k}=", v)
       end
 
-      @stubs             ||= []
-      @preload           ||= []
-      @processors        ||= ::Opal::Builder.processors
-      @path_reader       ||= PathReader.new(Opal.paths, extensions.map { |e| [".#{e}", ".js.#{e}"] }.flatten)
-      @prerequired       ||= []
-      @compiler_options  ||= Opal::Config.compiler_options
+      @stubs                    ||= []
+      @preload                  ||= []
+      @processors               ||= ::Opal::Builder.processors
+      @path_reader              ||= PathReader.new(Opal.paths, extensions.map { |e| [".#{e}", ".js.#{e}"] }.flatten)
+      @prerequired              ||= []
+      @compiler_options         ||= Opal::Config.compiler_options
+      @missing_require_severity ||= Opal::Config.missing_require_severity
 
       @processed = []
     end
@@ -83,6 +84,7 @@ def build(path, options = {})
     end
 
     def build_str(source, filename, options = {})
+      return if source.nil?
       path = path_from_filename(filename)
       asset = processor_for(source, filename, path, options)
       requires = preload + asset.requires + tree_requires(asset, path)
@@ -105,6 +107,7 @@ def initialize_copy(other)
       @path_reader = other.path_reader.dup
       @prerequired = other.prerequired.dup
       @compiler_options = other.compiler_options.dup
+      @missing_require_severity = other.missing_require_severity.to_sym
       @processed = other.processed.dup
     end
 
@@ -124,8 +127,8 @@ def append_paths(*paths)
 
     attr_reader :processed
 
-    attr_accessor :processors, :path_reader, :compiler_options,
-      :stubs, :prerequired, :preload
+    attr_accessor :processors, :path_reader, :stubs, :prerequired, :preload,
+      :compiler_options, :missing_require_severity
 
     private
 
@@ -160,7 +163,7 @@ def processor_for(source, filename, path, options)
                                            "source: #{source.inspect}, "\
                                            "processors: #{processors.inspect}"
                   )
-      processor.new(source, filename, compiler_options.merge(options))
+      processor.new(source, filename, @compiler_options.merge(options))
     end
 
     def read(path)
@@ -173,13 +176,13 @@ def read(path)
                   "\nAnd the following processors:\n" +
                   print_list[processors]
 
-        case compiler_options[:dynamic_require_severity]
-        when :raise   then raise MissingRequire, message
+        case missing_require_severity
+        when :error   then raise MissingRequire, message
         when :warning then warn message
-        else # noop
+        when :ignore  then # noop
         end
 
-        return "raise LoadError, #{message.inspect}"
+        nil
       end
     end
 
@@ -193,10 +196,13 @@ def process_require(filename, options)
 
       if source.nil?
         message = "can't find file: #{filename.inspect}"
-        case @compiler_options[:dynamic_require_severity]
-        when :error then raise LoadError, message
+        case missing_require_severity
+        when :error   then raise LoadError, message
         when :warning then warn "can't find file: #{filename.inspect}"
+        when :ignore  then # noop
         end
+
+        return # the handling is delegated to the runtime
       end
 
       path = path_from_filename(filename)

diff --git a/lib/opal/cli.rb b/lib/opal/cli.rb
@@ -8,7 +8,7 @@ module Opal
   class CLI
     attr_reader :options, :file, :compiler_options, :evals, :load_paths, :argv,
       :output, :requires, :gems, :stubs, :verbose, :runner_options,
-      :preload, :filename, :debug, :no_exit, :lib_only
+      :preload, :filename, :debug, :no_exit, :lib_only, :missing_require_severity
 
     class << self
       attr_accessor :stdout
@@ -36,8 +36,10 @@ def initialize(options = nil)
       @verbose     = options.delete(:verbose)    { false }
       @debug       = options.delete(:debug)      { false }
       @filename    = options.delete(:filename)   { @file && @file.path }
-
       @requires    = options.delete(:requires)   { [] }
+
+      @missing_require_severity = options.delete(:missing_require_severity) { Opal::Config.missing_require_severity }
+
       @requires.unshift('opal') unless options.delete(:skip_opal_require)
 
       @compiler_options = Hash[
@@ -77,7 +79,11 @@ def builder
     end
 
     def create_builder
-      builder = Opal::Builder.new stubs: stubs, compiler_options: compiler_options
+      builder = Opal::Builder.new(
+        stubs: stubs,
+        compiler_options: compiler_options,
+        missing_require_severity: missing_require_severity,
+      )
 
       # --include
       builder.append_paths(*load_paths)

diff --git a/lib/opal/cli_options.rb b/lib/opal/cli_options.rb
@@ -129,6 +129,14 @@ def initialize
         options[:dynamic_require_severity] = level.to_sym
       end
 
+      missing_require_levels = %w[error warning ignore]
+      on('--missing-require LEVEL', missing_require_levels,
+        'Set level of missing require severity.',
+        "(default: error, values: #{missing_require_levels.join(', ')})"
+      ) do |level|
+        options[:missing_require_severity] = level.to_sym
+      end
+
       on('-P', '--map FILE', 'Enable/Disable source map') do |file|
         options[:runner_options][:map_file] = file
       end

diff --git a/lib/opal/config.rb b/lib/opal/config.rb
@@ -98,7 +98,11 @@ def reset!
     # @return [true, false]
     config_option :tainting_stubs_enabled, true, compiler_option: :tainting
 
-    # Set the error severity for when a require can't be solved at compile time.
+    # Set the error severity for when a require can't be parsed at compile time.
+    #
+    # @example
+    #   # Opal code
+    #   require "foo" + some_dynamic_value
     #
     # - `:error` will raise an error at compile time
     # - `:warning` will print a warning on stderr at compile time
@@ -107,6 +111,19 @@ def reset!
     # @return [:error, :warning, :ignore]
     config_option :dynamic_require_severity, :warning, compiler_option: :dynamic_require_severity, valid_values: %i[error warning ignore]
 
+    # Set the error severity for when a required file can't be found at build time.
+    #
+    # @example
+    #   # Opal code
+    #   require "some_non_existen_file"
+    #
+    # - `:error` will raise an error at compile time
+    # - `:warning` will print a warning on stderr at compile time
+    # - `:ignore` will skip the require silently at compile time
+    #
+    # @return [:error, :warning, :ignore]
+    config_option :missing_require_severity, :error, valid_values: %i[error warning ignore]
+
     # Enable IRB support for making local variables across multiple compilations.
     #
     # @return [true, false]

diff --git a/opal/corelib/runtime.js b/opal/corelib/runtime.js
@@ -2206,7 +2206,11 @@
       var message  = 'cannot load such file -- ' + path;
 
       if (severity === "error") {
-        Opal.LoadError ? Opal.LoadError.$new(message) : function(){throw message}();
+        if (Opal.LoadError) {
+          throw Opal.LoadError.$new(message)
+        } else {
+          throw message
+        }
       }
       else if (severity === "warning") {
         console.warn('WARNING: LoadError: ' + message);

diff --git a/spec/lib/builder_spec.rb b/spec/lib/builder_spec.rb
@@ -86,4 +86,36 @@
     expect(builder.to_s).to include('TMP_foo_1.$$parameters = []')
   end
 
+  describe '#missing_require_severity' do
+    it 'defaults to warning' do
+      expect(builder.missing_require_severity).to eq(:error)
+    end
+
+    context 'when set to :warning' do
+      let(:options) { {missing_require_severity: :warning} }
+      it 'warns the user' do
+        expect(builder.missing_require_severity).to eq(:warning)
+        expect(builder).to receive(:warn) { |message| expect(message).to start_with(%{can't find file: "non-existen-file"}) }.at_least(1)
+        builder.build_str("require 'non-existen-file'", 'foo.rb')
+      end
+    end
+
+    context 'when set to :ignore' do
+      let(:options) { {missing_require_severity: :ignore} }
+      it 'does nothing' do
+        expect(builder.missing_require_severity).to eq(:ignore)
+        expect(builder).not_to receive(:warn)
+        expect{ builder.build_str("require 'non-existen-file'", 'foo.rb') }.not_to raise_error
+      end
+    end
+
+    context 'when set to :error' do
+      let(:options) { {missing_require_severity: :error} }
+      it 'raises MissingRequire' do
+        expect(builder.missing_require_severity).to eq(:error)
+        expect(builder).not_to receive(:warn)
+        expect{ builder.build_str("require 'non-existen-file'", 'foo.rb') }.to raise_error(described_class::MissingRequire)
+      end
+    end
+  end
 end
diff --git a/spec/lib/config_spec.rb b/spec/lib/config_spec.rb
@@ -23,6 +23,7 @@
     expect(described_class.freezing_stubs_enabled).to   eq(true)
     expect(described_class.tainting_stubs_enabled).to   eq(true)
     expect(described_class.dynamic_require_severity).to eq(:warning)
+    expect(described_class.missing_require_severity).to eq(:error)
     expect(described_class.irb_enabled).to              eq(false)
     expect(described_class.inline_operators_enabled).to eq(true)
     expect(described_class.source_map_enabled).to       eq(true)
@@ -36,6 +37,7 @@
     expect{ described_class.freezing_stubs_enabled   = :foobar }.to raise_error(ArgumentError)
     expect{ described_class.tainting_stubs_enabled   = :foobar }.to raise_error(ArgumentError)
     expect{ described_class.dynamic_require_severity = :foobar }.to raise_error(ArgumentError)
+    expect{ described_class.missing_require_severity = :foobar }.to raise_error(ArgumentError)
     expect{ described_class.irb_enabled              = :foobar }.to raise_error(ArgumentError)
     expect{ described_class.inline_operators_enabled = :foobar }.to raise_error(ArgumentError)
     expect{ described_class.source_map_enabled       = :foobar }.to raise_error(ArgumentError)
@@ -64,6 +66,15 @@
     expect{ described_class.dynamic_require_severity = :warning }.not_to raise_error
     expect(described_class.dynamic_require_severity).to eq(:warning)
 
+    expect{ described_class.missing_require_severity = :error }.not_to raise_error
+    expect(described_class.missing_require_severity).to eq(:error)
+
+    expect{ described_class.missing_require_severity = :ignore }.not_to raise_error
+    expect(described_class.missing_require_severity).to eq(:ignore)
+
+    expect{ described_class.missing_require_severity = :warning }.not_to raise_error
+    expect(described_class.missing_require_severity).to eq(:warning)
+
     expect{ described_class.freezing_stubs_enabled   = false }.not_to raise_error
     expect(described_class.freezing_stubs_enabled).to eq(false)
 

diff --git a/spec/lib/dependency_resolver_spec.rb b/spec/lib/dependency_resolver_spec.rb
@@ -1,31 +1,34 @@
 require 'lib/spec_helper'
 
 RSpec.describe Opal::Nodes::CallNode::DependencyResolver do
-  let(:compiler) { double(:compiler, :dynamic_require_severity => :none) }
+  let(:compiler) { double(:compiler, dynamic_require_severity: :warning) }
 
   it "resolves simple strings to themselves" do
     expect(resolve s(:str, 'foo')).to eq('foo')
   end
 
   context "using a dynamic segment not supported" do
     it "raises a compiler error when severity is :error" do
-      compiler = double(:compiler, :dynamic_require_severity => :error)
-      expect(compiler).to receive(:error).once
-      expect(compiler).to receive(:dynamic_require_severity).once
+      compiler = double(:compiler, dynamic_require_severity: :error)
+      expect(compiler).to     receive(:dynamic_require_severity).once
+      expect(compiler).not_to receive(:warning)
+      expect(compiler).to     receive(:error).once
       described_class.new(compiler, s(:self)).resolve
     end
 
     it "produces a compiler warning when severity is :warning" do
-      compiler = double(:compiler, :dynamic_require_severity => :warning)
-      expect(compiler).to receive(:warning).once
-      expect(compiler).to receive(:dynamic_require_severity).once
+      compiler = double(:compiler, dynamic_require_severity: :warning)
+      expect(compiler).to     receive(:dynamic_require_severity).once
+      expect(compiler).to     receive(:warning).once
+      expect(compiler).not_to receive(:error)
       described_class.new(compiler, s(:self)).resolve
     end
 
-    it "does not produce a warning or error for other options" do
-      compiler = double(:compiler, :dynamic_require_severity => :foo)
-      expect(compiler).to_not receive(:warning)
-      expect(compiler).to_not receive(:error)
+    it "does not produce a warning when severity is :ignore" do
+      compiler = double(:compiler, dynamic_require_severity: :ignore)
+      expect(compiler).to     receive(:dynamic_require_severity).once
+      expect(compiler).not_to receive(:warning)
+      expect(compiler).not_to receive(:error)
       described_class.new(compiler, s(:self)).resolve
     end
   end

diff --git a/spec/lib/spec_helper.rb b/spec/lib/spec_helper.rb
@@ -78,6 +78,9 @@
     # unless a formatter has already been configured
     # (e.g. via a command-line flag).
     config.default_formatter = 'doc'
+
+    # Show full backtrace
+    config.backtrace_exclusion_patterns = []
   else
     # The RSpec specs are stored in spec/lib.
     config.pattern = 'spec/lib/**/*_spec.rb'

diff --git a/tasks/benchmarking.rake b/tasks/benchmarking.rake
@@ -107,7 +107,7 @@ namespace :bench do
     raise ArgumentError, "no files provided" if files.empty?
     puts "=== Files: #{files.join ', '}"
     files.each do |bm_path|
-      sh "bundle exec opal -ropal/platform -gbenchmark-ips -rbenchmark/ips -A #{bm_path}"
+      sh "bundle exec opal --dynamic-require ignore --missing-require ignore -ropal/platform -gbenchmark-ips -rbenchmark/ips -A #{bm_path}"
     end
   end
 end
diff --git a/tasks/building.rake b/tasks/building.rake
@@ -17,6 +17,7 @@ task :dist do
   Opal::Config.arity_check_enabled = false
   Opal::Config.const_missing_enabled = false
   Opal::Config.dynamic_require_severity = :warning
+  Opal::Config.missing_require_severity = :error
 
   # Hike gem is required to build opal-builder
   Opal.use_gem 'hike'