Browse files

Added override warnings for Module#{public,private,protected,module_f…

…unction}
  • Loading branch information...
1 parent 5d737eb commit 8d46a07e079b0da0614a0ce6297c56ae91560f3e Michael Edgar committed Aug 22, 2011
Showing with 146 additions and 75 deletions.
  1. +1 −2 features/support/env.rb
  2. +3 −1 lib/laser.rb
  3. +1 −1 lib/laser/analysis/bindings.rb
  4. +1 −1 lib/laser/analysis/bootstrap/bootstrap.rb
  5. +0 −4 lib/laser/analysis/bootstrap/laser_class.rb
  6. +24 −4 lib/laser/analysis/bootstrap/laser_method.rb
  7. +5 −1 lib/laser/analysis/bootstrap/laser_module.rb
  8. +1 −1 lib/laser/analysis/control_flow.rb
  9. +1 −1 lib/laser/analysis/control_flow/alias_analysis.rb
  10. +1 −1 lib/laser/analysis/control_flow/basic_block.rb
  11. +1 −1 lib/laser/analysis/control_flow/cfg_builder.rb
  12. +2 −2 lib/laser/analysis/control_flow/cfg_instruction.rb
  13. +2 −2 lib/laser/analysis/control_flow/constant_propagation.rb
  14. +1 −1 lib/laser/analysis/control_flow/control_flow_graph.rb
  15. +1 −2 lib/laser/analysis/control_flow/method_call_search.rb
  16. +2 −2 lib/laser/analysis/control_flow/simulation.rb
  17. +1 −1 lib/laser/analysis/control_flow/static_single_assignment.rb
  18. +1 −1 lib/laser/analysis/control_flow/unused_variables.rb
  19. +1 −1 lib/laser/analysis/control_flow/yield_properties.rb
  20. +5 −1 lib/laser/analysis/errors.rb
  21. +1 −1 lib/laser/analysis/laser_utils.rb
  22. +35 −0 lib/laser/analysis/override_safety_info.rb
  23. +1 −1 lib/laser/analysis/signature.rb
  24. +1 −1 lib/laser/annotation_parser/annotations_parser.rb
  25. +1 −1 lib/laser/annotation_parser/class_annotations_parser.rb
  26. +1 −2 lib/laser/annotation_parser/overload_parser.rb
  27. +1 −1 lib/laser/annotation_parser/parsers.rb
  28. +1 −1 lib/laser/types/types.rb
  29. +1 −1 lib/laser/warning.rb
  30. +1 −1 lib/laser/warnings/line_length.rb
  31. +1 −1 spec/analysis_specs/argument_expansion_spec.rb
  32. +1 −1 spec/analysis_specs/control_flow_specs/constant_propagation_spec.rb
  33. +26 −12 spec/analysis_specs/control_flow_specs/improper_override_spec.rb
  34. +1 −2 spec/analysis_specs/control_flow_specs/raise_properties_spec.rb
  35. +1 −1 spec/analysis_specs/control_flow_specs/return_type_inference_spec.rb
  36. +1 −1 spec/analysis_specs/control_flow_specs/simulation_spec.rb
  37. +1 −1 spec/analysis_specs/control_flow_specs/spec_helper.rb
  38. +1 −1 spec/analysis_specs/control_flow_specs/unreachability_analysis_spec.rb
  39. +1 −1 spec/analysis_specs/control_flow_specs/yield_properties_spec.rb
  40. +1 −1 spec/analysis_specs/laser_class_spec.rb
  41. +1 −1 spec/analysis_specs/scope_annotation_spec.rb
  42. +1 −1 spec/analysis_specs/scope_spec.rb
  43. +1 −1 spec/analysis_specs/sexp_extension_specs/constant_extraction_spec.rb
  44. +1 −1 spec/analysis_specs/sexp_extension_specs/spec_helper.rb
  45. +1 −1 spec/annotation_parser_specs/annotations_parser_spec.rb
  46. +1 −1 spec/annotation_parser_specs/class_annotation_parser_spec.rb
  47. +1 −1 spec/annotation_parser_specs/overload_parser_spec.rb
  48. +1 −1 spec/annotation_parser_specs/structural_parser_spec.rb
  49. +1 −1 spec/standard_library/set_spec.rb
  50. +1 −1 spec/standard_library/standard_library_spec.rb
  51. +1 −1 spec/type_specs/types_spec.rb
  52. +1 −1 spec/warning_spec.rb
View
3 features/support/env.rb
@@ -10,5 +10,4 @@ def swizzling_io
$stdout.string
ensure
$stdout = old_stdout
-end
-
+end
View
4 lib/laser.rb
@@ -70,6 +70,7 @@ def self.debug?
require 'laser/analysis/bootstrap/laser_proc'
require 'laser/analysis/bootstrap/laser_method'
require 'laser/analysis/bootstrap/dispatch_results'
+require 'laser/analysis/override_safety_info'
require 'laser/analysis/laser_utils.rb'
require 'laser/analysis/protocol_registry'
require 'laser/analysis/scope'
@@ -103,4 +104,5 @@ def self.debug?
require 'laser/version'
# All methods created from the stdlib should never be marked as unused.
-Laser::Analysis::Bootstrap.load_standard_library
+Laser::Analysis::Bootstrap.load_standard_library
+# override info needs stdlib methods to be instantiated
View
2 lib/laser/analysis/bindings.rb
@@ -141,4 +141,4 @@ def is_block?
end
end
end
-end
+end
View
2 lib/laser/analysis/bootstrap/bootstrap.rb
@@ -305,4 +305,4 @@ def self.stub_global_type(name, type)
end
end
end
-end
+end
View
4 lib/laser/analysis/bootstrap/laser_class.rb
@@ -44,10 +44,6 @@ def remove_subclass!(other)
subclasses.delete other
end
- def parent
- @superclass
- end
-
def superclass
current = @superclass
while current
View
28 lib/laser/analysis/bootstrap/laser_method.rb
@@ -5,9 +5,9 @@ module Analysis
class LaserMethod
extend ModuleExtensions
cattr_accessor_with_default :default_dispatched, true
- attr_reader :name, :proc, :arglist
+ attr_reader :name, :proc, :arglist, :owner
alias arguments arglist
- attr_accessor :owner, :arity
+ attr_accessor :arity
def initialize(name, base_proc)
@name = name
@@ -25,6 +25,16 @@ def initialize(name, base_proc)
yield self if block_given?
end
+ def owner=(mod)
+ @owner = mod
+ verify_override_safety
+ end
+
+ # Returns the method this method overrides: what gets called by `super`.
+ def super_method
+ owner.parent && owner.parent.instance_method(name)
+ end
+
################## Potentially Annotated Properties ######################
%w(special pure builtin predictable mutation annotated_return annotated_yield_usage
@@ -160,18 +170,28 @@ def arg_types_unify_with_annotations?(arg_types)
def check_return_type_against_expectations(return_type)
if (expectation = Types::EXPECTATIONS[self.name]) &&
!Types.subtype?(return_type, expectation)
- @proc.ast_node.add_error(ImproperOverloadTypeError.new(
+ @proc.ast_node.add_error(ImproperOverrideTypeError.new(
"All methods named #{self.name} should return a subtype of #{expectation.inspect}",
@proc.ast_node))
elsif self.name.end_with?('?')
if !Types.subtype?(return_type, Types::BOOL_OR_NIL)
- @proc.ast_node.add_error(ImproperOverloadTypeError.new(
+ @proc.ast_node.add_error(ImproperOverrideTypeError.new(
"All methods whose name ends in ? should return a subtype of TrueClass | FalseClass | NilClass",
@proc.ast_node))
end
end
end
+ def verify_override_safety
+ overridden = super_method
+ return unless overridden
+ if OverrideSafetyInfo.warn_on_any_override?(overridden)
+ @proc.ast_node.add_error(DangerousOverrideError.new(
+ OverrideSafetyInfo.warning_message(overridden),
+ @proc.ast_node))
+ end
+ end
+
def dispatched?
@dispatched
end
View
6 lib/laser/analysis/bootstrap/laser_module.rb
@@ -209,7 +209,11 @@ def set_visibility!(method, visibility)
def superclass=(new_superclass)
@superclass = new_superclass
end
-
+
+ def parent
+ @superclass
+ end
+
# The set of all superclasses (including the class itself)
def ancestors
if @superclass.nil?
View
2 lib/laser/analysis/control_flow.rb
@@ -25,4 +25,4 @@ def self.perform_cfg_analysis(tree, text, opts={})
end
end
end
-end
+end
View
2 lib/laser/analysis/control_flow/alias_analysis.rb
@@ -28,4 +28,4 @@ def value_aliases(value_to_match)
end
end # ControlFlow
end # Analysis
-end #
+end #
View
2 lib/laser/analysis/control_flow/basic_block.rb
@@ -102,4 +102,4 @@ def instructions
end
end
end
-end
+end
View
2 lib/laser/analysis/control_flow/cfg_builder.rb
@@ -2503,4 +2503,4 @@ def start_block(block)
end
end
end
-end
+end
View
4 lib/laser/analysis/control_flow/cfg_instruction.rb
@@ -113,7 +113,7 @@ def possible_methods(opts)
self[2].expr_type.matching_methods(self[3])
end
else
- [opts[:method].owner.parent.instance_method(opts[:method].name)]
+ [opts[:method].super_method]
end
end
@@ -187,4 +187,4 @@ def operand_range
end
end
end
-end
+end
View
4 lib/laser/analysis/control_flow/constant_propagation.rb
@@ -346,7 +346,7 @@ def cpa_dispatches(receiver, instruction, method, opts)
receiver.expr_type.member_types.map do |type|
[type, type.matching_methods(method)]
end
- else
+ else # super, super_vararg
dispatches = instruction.possible_methods(opts)
receiver.expr_type.member_types.map do |type|
[type, dispatches]
@@ -654,4 +654,4 @@ def validate_tuple_expectation(instruction)
end # ConstantPropagation
end # ControlFlow
end # Analysis
-end #
+end #
View
2 lib/laser/analysis/control_flow/control_flow_graph.rb
@@ -367,4 +367,4 @@ def kill_unexecuted_edges(remove=false)
end
end
end
-end
+end
View
3 lib/laser/analysis/control_flow/method_call_search.rb
@@ -22,5 +22,4 @@ def find_method_calls(method_to_find, opts)
end
end # ControlFlow
end # Analysis
-end #
-
+end #
View
4 lib/laser/analysis/control_flow/simulation.rb
@@ -182,7 +182,7 @@ def simulate_call_instruction(insn, opts)
method = klass.instance_method(insn[3]) # normal method dispatch
else
# super method dispatch
- method = opts[:method].owner.parent.instance_method(opts[:method].name)
+ method = opts[:method].super_method
end
if !method
simulate_method_missing(insn, klass)
@@ -387,4 +387,4 @@ def simulate_module_eval(receiver, args, block)
end # Simulation
end # ControlFlow
end # Analysis
-end # Laser
+end # Laser
View
2 lib/laser/analysis/control_flow/static_single_assignment.rb
@@ -182,4 +182,4 @@ def new_ssa_name(temp)
end
end
end
-end
+end
View
2 lib/laser/analysis/control_flow/unused_variables.rb
@@ -88,4 +88,4 @@ def killable_with_unused_target?(insn)
end
end # ControlFlow
end # Analysis
-end #
+end #
View
2 lib/laser/analysis/control_flow/yield_properties.rb
@@ -100,4 +100,4 @@ def potential_block_calls(block_value = nil, opts)
end
end # ControlFlow
end # Analysis
-end #
+end #
View
6 lib/laser/analysis/errors.rb
@@ -117,7 +117,11 @@ class UnnecessaryBlockError < Laser::Error
severity TRICKY_ERROR
end
- class ImproperOverloadTypeError < Laser::Error
+ class ImproperOverrideTypeError < Laser::Error
+ severity TRICKY_ERROR
+ end
+
+ class DangerousOverrideError < Laser::Error
severity TRICKY_ERROR
end
View
2 lib/laser/analysis/laser_utils.rb
@@ -15,4 +15,4 @@ def normal_class_for(arg)
end
end
end
-end
+end
View
35 lib/laser/analysis/override_safety_info.rb
@@ -0,0 +1,35 @@
+module Laser
+ module Analysis
+ module OverrideSafetyInfo
+ def self.warn_on_any_override?(method)
+ do_not_override.include?(method)
+ end
+
+ def self.warning_message(method)
+ do_not_override[method]
+ end
+
+ def self.do_not_override
+ return @do_not_override if @do_not_override
+ result = Hash[
+ ClassRegistry['Module'].instance_method(:public),
+ 'Overriding Module#public breaks its zero-argument lexically-scoped behavior.',
+ ClassRegistry['Module'].instance_method(:private),
+ 'Overriding Module#private breaks its zero-argument lexically-scoped behavior.',
+ ClassRegistry['Module'].instance_method(:protected),
+ 'Overriding Module#protected breaks its zero-argument lexically-scoped behavior.',
+ ClassRegistry['Module'].instance_method(:module_function),
+ 'Overriding Module#module_function breaks its zero-argument lexically-scoped behavior.'
+ ]
+ unless result[nil] # if all keys existed
+ @do_not_override = result
+ end
+ result
+ end
+ class << self
+ private :do_not_override
+ @do_not_override = nil
+ end
+ end
+ end
+end
View
2 lib/laser/analysis/signature.rb
@@ -73,4 +73,4 @@ module Signature
extend ArgumentListHandling
end
end
-end
+end
View
2 lib/laser/annotation_parser/annotations_parser.rb
@@ -745,4 +745,4 @@ class AnnotationParser < Treetop::Runtime::CompiledParser
end
end
-end
+end
View
2 lib/laser/annotation_parser/class_annotations_parser.rb
@@ -651,4 +651,4 @@ class ClassParser < Treetop::Runtime::CompiledParser
end
end
-end
+end
View
3 lib/laser/annotation_parser/overload_parser.rb
@@ -163,5 +163,4 @@ class OverloadParser < Treetop::Runtime::CompiledParser
end
end
-end
-
+end
View
2 lib/laser/annotation_parser/parsers.rb
@@ -3,4 +3,4 @@
require_relative 'class_annotations_parser'
require_relative 'structural_parser'
require_relative 'overload_parser'
-require_relative 'annotations_parser'
+require_relative 'annotations_parser'
View
2 lib/laser/types/types.rb
@@ -433,4 +433,4 @@ def raise_type_for_types(self_type, arg_types = [], block_type = nil)
'to_ary' => Types::ARRAY,
'!' => Types::BOOLEAN }
end
-end
+end
View
2 lib/laser/warning.rb
@@ -148,4 +148,4 @@ def self.options
Dir[File.expand_path(File.join(File.dirname(__FILE__), 'warnings', '**', '*.rb'))].each do |file|
load file
-end
+end
View
2 lib/laser/warnings/line_length.rb
@@ -112,4 +112,4 @@ def LineLengthWarning(size)
(@table ||= {})[size] ||= LineLengthCustomSeverity(size, 3)
end
module_function :LineLengthMaximum, :LineLengthWarning, :LineLengthCustomSeverity
-end
+end
View
2 spec/analysis_specs/argument_expansion_spec.rb
@@ -110,4 +110,4 @@
expansion.constant_values.should == [:a, :b]
end
end
-end
+end
View
2 spec/analysis_specs/control_flow_specs/constant_propagation_spec.rb
@@ -557,4 +557,4 @@ def foo_blockitized
EOF
g.should have_constant('z').with_value(['aa', 'bb', 'cc'])
end
-end
+end
View
38 spec/analysis_specs/control_flow_specs/improper_override_spec.rb
@@ -14,7 +14,7 @@ def #{method}
return_type_for_types(
ClassRegistry["OverI1#{method}"].as_type) # force calculation
ClassRegistry["OverI1#{method}"].instance_method(method).proc.ast_node.should(
- have_error(ImproperOverloadTypeError).with_message(/#{method}/))
+ have_error(ImproperOverrideTypeError).with_message(/#{method}/))
end
it "should not warn against a method named #{method} that always returns a string" do
@@ -29,7 +29,7 @@ def #{method}
return_type_for_types(
ClassRegistry["OverI3#{method}"].as_type) # force calculation
ClassRegistry["OverI3#{method}"].instance_method(method).proc.ast_node.should_not(
- have_error(ImproperOverloadTypeError))
+ have_error(ImproperOverrideTypeError))
end
end
@@ -46,7 +46,7 @@ def #{method}
return_type_for_types(
ClassRegistry["OverI2#{method}"].as_type) # force calculation
ClassRegistry["OverI2#{method}"].instance_method(method).proc.ast_node.should(
- have_error(ImproperOverloadTypeError).with_message(/#{method}/))
+ have_error(ImproperOverrideTypeError).with_message(/#{method}/))
end
it "should not warn against a method named #{method} that always returns an integer" do
@@ -61,7 +61,7 @@ def #{method}
return_type_for_types(
ClassRegistry["OverI3#{method}"].as_type) # force calculation
ClassRegistry["OverI3#{method}"].instance_method(method).proc.ast_node.should_not(
- have_error(ImproperOverloadTypeError))
+ have_error(ImproperOverrideTypeError))
end
end
@@ -78,7 +78,7 @@ def #{method}
return_type_for_types(
ClassRegistry["OverI2#{method}"].as_type) # force calculation
ClassRegistry["OverI2#{method}"].instance_method(method).proc.ast_node.should(
- have_error(ImproperOverloadTypeError).with_message(/#{method}/))
+ have_error(ImproperOverrideTypeError).with_message(/#{method}/))
end
it "should not warn against a method named #{method} that always returns an array" do
@@ -93,7 +93,7 @@ def #{method}
return_type_for_types(
ClassRegistry["OverI3#{method}"].as_type) # force calculation
ClassRegistry["OverI3#{method}"].instance_method(method).proc.ast_node.should_not(
- have_error(ImproperOverloadTypeError))
+ have_error(ImproperOverrideTypeError))
end
end
@@ -109,7 +109,7 @@ def to_f
return_type_for_types(
ClassRegistry["OverI2to_f"].as_type) # force calculation
ClassRegistry["OverI2to_f"].instance_method(:to_f).proc.ast_node.should(
- have_error(ImproperOverloadTypeError).with_message(/to_f/))
+ have_error(ImproperOverrideTypeError).with_message(/to_f/))
end
@@ -125,7 +125,7 @@ def to_f
return_type_for_types(
ClassRegistry["OverI3"].as_type) # force calculation
ClassRegistry["OverI3"].instance_method(:to_f).proc.ast_node.should_not(
- have_error(ImproperOverloadTypeError))
+ have_error(ImproperOverrideTypeError))
end
it "should warn against a method named ! that doesn't always return a boolean" do
@@ -140,7 +140,7 @@ def !
return_type_for_types(
ClassRegistry["OverI2"].as_type) # force calculation
ClassRegistry["OverI2"].instance_method(:!).proc.ast_node.should(
- have_error(ImproperOverloadTypeError).with_message(/\!/))
+ have_error(ImproperOverrideTypeError).with_message(/\!/))
end
it "should not warn against a method named ! that always returns a boolean" do
@@ -155,7 +155,7 @@ def !
return_type_for_types(
ClassRegistry["OverI3"].as_type) # force calculation
ClassRegistry["OverI3"].instance_method(:!).proc.ast_node.should_not(
- have_error(ImproperOverloadTypeError))
+ have_error(ImproperOverrideTypeError))
end
it 'should warn when a method whose name ends in ? does not return a bool | nil' do
@@ -170,7 +170,7 @@ def silly?(x, y)
return_type_for_types(
ClassRegistry["OverI4"].as_type, [Types::FIXNUM, Types::FIXNUM])
ClassRegistry["OverI4"].instance_method(:silly?).proc.ast_node.should(
- have_error(ImproperOverloadTypeError))
+ have_error(ImproperOverrideTypeError))
end
it 'should not warn when a method whose name ends in ? does return a bool | nil' do
@@ -185,6 +185,20 @@ def silly?(x, y)
return_type_for_types(
ClassRegistry["OverI5"].as_type, [Types::FIXNUM, Types::FIXNUM])
ClassRegistry["OverI5"].instance_method(:silly?).proc.ast_node.should_not(
- have_error(ImproperOverloadTypeError))
+ have_error(ImproperOverrideTypeError))
+ end
+
+ %w(public private protected module_function).each do |method|
+ it "should warn when the visibility method Module##{method} is overridden" do
+ g = cfg <<-EOF
+class OverI6
+ def self.#{method}(*args)
+ super
+ end
+end
+EOF
+ ClassRegistry["OverI6"].singleton_class.instance_method(method).proc.ast_node.should(
+ have_error(DangerousOverrideError))
+ end
end
end
View
3 spec/analysis_specs/control_flow_specs/raise_properties_spec.rb
@@ -306,5 +306,4 @@ def foo(*args)
[Types::STRING, Types::FIXNUM, Types::BIGNUM,
Types::ARRAY, Types::HASH]).should == Frequency::ALWAYS
end
-end
-
+end
View
2 spec/analysis_specs/control_flow_specs/return_type_inference_spec.rb
@@ -413,4 +413,4 @@ def foo(x)
Types::FIXNUM, ClassRegistry['Symbol'].as_type,
Types::FIXNUM, ClassRegistry['Symbol'].as_type]))
end
-end
+end
View
2 spec/analysis_specs/control_flow_specs/simulation_spec.rb
@@ -155,4 +155,4 @@ def foo(x)
# tree.all_errors.first.should be_a(TopLevelSimulationRaised)
# tree.all_errors.first.error.should be_a(ControlFlow::Simulation::SimulationNonterminationError)
# end
-end
+end
View
2 spec/analysis_specs/control_flow_specs/spec_helper.rb
@@ -107,4 +107,4 @@ def cfg_method(input)
graph = cfg_builder.build
graph.analyze
graph
-end
+end
View
2 spec/analysis_specs/control_flow_specs/unreachability_analysis_spec.rb
@@ -73,4 +73,4 @@ def foo
EOF
g.should have_error(Laser::DeadCodeWarning).on_line(6)
end
-end
+end
View
2 spec/analysis_specs/control_flow_specs/yield_properties_spec.rb
@@ -370,4 +370,4 @@ def each
method = ClassRegistry['YP10'].instance_method(:each)
method.yield_type.should be :optional
end
-end
+end
View
2 spec/analysis_specs/laser_class_spec.rb
@@ -319,4 +319,4 @@
@method.name.should == 'foobar'
end
end
-end
+end
View
2 spec/analysis_specs/scope_annotation_spec.rb
@@ -1010,4 +1010,4 @@ def initialize(name, value, kind, default_value = nil)
arg_binding.instance_method(:initialize).arity.should == (3..4)
end
end
-end
+end
View
2 spec/analysis_specs/scope_spec.rb
@@ -123,4 +123,4 @@ def add_scope_instance_variables(klass)
}.to raise_error(Scope::ScopeLookupFailure)
end
end
-end
+end
View
2 spec/analysis_specs/sexp_extension_specs/constant_extraction_spec.rb
@@ -306,4 +306,4 @@
end
end
end
-end
+end
View
2 spec/analysis_specs/sexp_extension_specs/spec_helper.rb
@@ -1 +1 @@
-require_relative '../spec_helper'
+require_relative '../spec_helper'
View
2 spec/annotation_parser_specs/annotations_parser_spec.rb
@@ -86,4 +86,4 @@
result.should_not be_type
end
end
-end
+end
View
2 spec/annotation_parser_specs/class_annotation_parser_spec.rb
@@ -117,4 +117,4 @@
end
end
end
-end
+end
View
2 spec/annotation_parser_specs/overload_parser_spec.rb
@@ -36,4 +36,4 @@
Types::UnionType.new([Types::FLOAT, Types::NILCLASS])]))
end
end
-end
+end
View
2 spec/annotation_parser_specs/structural_parser_spec.rb
@@ -64,4 +64,4 @@
'#write'.should parse_to(Types::StructuralType.new('write', [], []))
end
end
-end
+end
View
2 spec/standard_library/set_spec.rb
@@ -28,4 +28,4 @@
ClassRegistry['Set'].instance_method(method).should_not be_nil
end
end
-end
+end
View
2 spec/standard_library/standard_library_spec.rb
@@ -299,4 +299,4 @@
it_should_behave_like 'a class'
end
-end
+end
View
2 spec/type_specs/types_spec.rb
@@ -130,4 +130,4 @@
end
end
end
-end
+end
View
2 spec/warning_spec.rb
@@ -92,4 +92,4 @@ def self.set_type
klass.short_name.should =~ /SI\d/
end
end
-end
+end

0 comments on commit 8d46a07

Please sign in to comment.