Skip to content

Commit

Permalink
Improve error message consistency in BuilderDsl-adjacent code; Write
Browse files Browse the repository at this point in the history
specs for IdentifierStack and fix a bug; Change Navigation#elementn to
strictly validate its arguments, even when corresponding data is not
present; Fix validation in Navigation#iterate (#126); Write many more
specs
  • Loading branch information
kputnam committed Feb 12, 2019
1 parent ec7e464 commit 1235c80
Show file tree
Hide file tree
Showing 14 changed files with 997 additions and 162 deletions.
4 changes: 2 additions & 2 deletions lib/stupidedi/parser/builder_dsl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def strict?
# @return [BuilderDsl]
def segment!(name, position, *elements)
segment_tok = mksegment_tok(@reader.segment_dict, name, elements, position)
machine, reader = @machine.insert(segment_tok, @reader)
machine, reader = @machine.insert(segment_tok, @strict, @reader)

if @strict
unless machine.deterministic?
Expand Down Expand Up @@ -124,7 +124,7 @@ def critique(zipper, recursive = false, position = false)
"forbidden #{zipper.node.descriptor} is present at #{zipper.node.position.inspect}"
elsif not zipper.node.allowed?
raise Exceptions::ParseError,
"value #{zipper.node.to_s} not allowed in #{zipper.node.descriptor} at #{zipper.node.position.inspect}"
"value #{zipper.node.to_s} is not allowed in #{zipper.node.descriptor} at #{zipper.node.position.inspect}"
elsif zipper.node.too_long?
raise Exceptions::ParseError,
"value is too long in #{zipper.node.descriptor} at #{zipper.node.position.inspect}"
Expand Down
40 changes: 21 additions & 19 deletions lib/stupidedi/parser/constraint_table.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ module Parser
#
class ConstraintTable
# @return [Array<Instruction>]
abstract :matches, :args => %w(segment_tok mode)
abstract :matches, :args => %w(segment_tok strict mode)

# @return [Array<Instruction>]
attr_reader :instructions
Expand All @@ -29,6 +29,7 @@ def copy(changes = {})
end

# @return [void]
# :nocov:
def pretty_print(q)
name = self.class.name.split("::").last
q.text "#{name}.build"
Expand All @@ -43,6 +44,7 @@ def pretty_print(q)
end
end
end
# :nocov:

# @todo
def critique(segment_tok, segment_uses)
Expand Down Expand Up @@ -90,19 +92,19 @@ def matches(segment_tok, strict, mode)
# Chooses the {Instruction} that pops the greatest number of states.
#
class Deepest < ConstraintTable
def initialize(instructions)
@instructions = instructions
end

# @return [Array<Instruction>]
def matches(segment_tok, strict, mode)
@__matches ||= begin
deepest = @instructions.map(&:pop_count).max
@instructions.select{|i| i.pop_count == deepest }.tap do |xs|
critique(segment_tok, xs.map(&:segment_use)) if strict
end
end
end
# def initialize(instructions)
# @instructions = instructions
# end

# # @return [Array<Instruction>]
# def matches(segment_tok, strict, mode)
# @__matches ||= begin
# deepest = @instructions.map(&:pop_count).max
# @instructions.select{|i| i.pop_count == deepest }.tap do |xs|
# critique(segment_tok, xs.map(&:segment_use)) if strict
# end
# end
# end
end

# Chooses the subset of {Instruction} values based on the distinguishing
Expand Down Expand Up @@ -141,10 +143,10 @@ def matches(segment_tok, strict, mode)
else
if strict
designator = "#{segment_tok.id}#{"%02d" % (n + 1)}"
designator = designator + "-%02d" % m unless m.nil?
designator = designator + "-%02d" % (m + 1) unless m.nil?

raise ArgumentError,
"#{value.inspect} is not allowed in #{designator}"
"value #{value.to_s} is not allowed in element #{designator}"
end
end
end
Expand Down Expand Up @@ -177,11 +179,11 @@ def matches(segment_tok, strict, mode)
else
# This value isn't compatible with any instruction
if strict
designator = "#{segment_tok.id}#{"%02d" % n}"
designator = designator + "-%02d" % m unless m.nil?
designator = "#{segment_tok.id}#{"%02d" % (n + 1)}"
designator = designator + "-%02d" % (m + 1) unless m.nil?

raise ArgumentError,
"#{value.inspect} is not allowed in #{designator}"
"value #{value.to_s} is not allowed in element #{designator}"
end
end
end
Expand Down
10 changes: 5 additions & 5 deletions lib/stupidedi/parser/generation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def read(reader, options = {})
while reader_e.defined?
reader_e = reader_e.flatmap do |segment_tok, reader_|
machine, reader__ =
machine.insert(segment_tok, reader_)
machine.insert(segment_tok, false, reader_)

if machine.active.length <= limit
reader__.read_segment
Expand All @@ -54,10 +54,10 @@ def read(reader, options = {})
end

# @return [(StateMachine, Reader::TokenReader)]
def insert(segment_tok, reader)
def insert(segment_tok, strict, reader)
active = @active.flat_map do |zipper|
state = zipper.node
instructions = state.instructions.matches(segment_tok)
instructions = state.instructions.matches(segment_tok, strict, :insert)

if instructions.empty?
zipper.append(FailureState.mksegment(segment_tok, state)).cons
Expand Down Expand Up @@ -116,8 +116,8 @@ def execute(op, zipper, reader, segment_tok)
parent = state.node.copy \
:zipper => value,
:children => [],
:separators => reader.separators,
:segment_dict => reader.segment_dict,
:separators => reader.try(&:separators),
:segment_dict => reader.try(&:segment_dict),
:instructions => table.pop(op.pop_count).drop(op.drop_count)

# Note, `state` is a cursor pointing at a state, while `parent`
Expand Down
6 changes: 3 additions & 3 deletions lib/stupidedi/parser/identifier_stack.rb
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ def id
end

def hl
HL.new(self, @hl_sequence, @lx_sequence).tap { @hl_sequence.succ! }
HL.new(self, @hl_sequence.succ!, @lx_sequence)
end

def lx
Expand Down Expand Up @@ -216,7 +216,7 @@ def initialize(parent, hl_sequence, lx_sequence)
end

def hl
HL.new(self, @sequence.succ!, @lx_sequence)
HL.new(self, @hl_sequence.succ!, @lx_sequence)
end

def lx
Expand All @@ -226,7 +226,7 @@ def lx
# Parent HL number (HL02)
#
# @return [Integer]
def parent_id
def parent_hl
if @parent.is_a?(HL)
@parent.id
else
Expand Down
163 changes: 111 additions & 52 deletions lib/stupidedi/parser/navigation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -117,80 +117,131 @@ def segmentn
#
# @return [Either<Zipper::AbstractCursor<Values::AbstractElementVal>>]
def element(m, n = nil, o = nil)
segment.flatmap do |s|
unless m >= 1
raise ArgumentError,
"argument must be positive"
end
if m <= 0 or (n || 1) <= 0 or (o || 1) <= 0
raise ArgumentError,
"all arguments must be positive"
end

if n.nil? and not o.nil?
raise ArgumentError,
"third argument cannot be present unless second argument is present"
end

segment.flatmap do |s|
if s.node.invalid?
# InvalidSegmentVal doesn't have child AbstractElementVals, its
# children are SimpleElementTok, CompositeElementTok, etc, which
# are not parsed values.
return Either.failure("invalid segment")
end

designator = s.node.id.to_s
definition = s.node.definition
length = definition.element_uses.length
segment_id = s.node.id.to_s
segment_def = s.node.definition
descriptor = segment_id

unless m <= length
unless m <= segment_def.element_uses.length
raise ArgumentError,
"#{designator} segment has only #{length} elements"
"segment #{descriptor} has only #{segment_def.element_uses.length} elements"
end

designator = designator + "%02d" % m
value = s.child(m - 1)
element_use = segment_def.element_uses.at(m - 1)
element_def = element_use.definition
element_zip = s.child(m - 1)

if n.nil?
return Either.success(value)
elsif value.node.repeated?
unless n >= 1
return Either.success(element_zip)

elsif element_use.composite? and not element_use.repeatable?
# m: element of segment
# n: component of composite element
# o: occurence of repeated component
descriptor = "%s%02d" % [segment_id, m]
components = element_def.component_uses.length
unless n <= components
raise ArgumentError,
"argument must be positive"
"composite element #{descriptor} only has #{components} components"
end

limit = value.node.usage.repeat_count
unless limit.include?(n)
component_use = element_def.component_uses.at(n - 1)

if o.nil?
# This is a component of a composite element
return Either.success(element_zip.child(n - 1))

# @todo: There currently doesn't seem to be any instances of this in
# the real world (a composite element that has a component that can
# repeat), but perhaps this will happen in the future.
#
# elsif component_use.repeatable?
# repeat_count = component_use.repeat_count
# occurs_count = component_val.children.length
# descriptor = "%s%02d-%02d" % [segment_id, m, n]
# unless repeat_count.include?(o)
# raise ArgumentError,
# "repeatable component element #{descriptor} can only occur #{repeat_count.max} times"
# end
# component_zip = element_zip.child(n - 1)
# if component_zip.node.blank?
# return Either.failure("repeating component element #{descriptor} is blank")
# elsif occurs_count < n
# return Either.failure("repeating component element #{descriptor} only occurs #{occurs_count} times")
# else
# return Either.success(component_zip.child(o - 1))
# end

else
descriptor = "%s%02d-%02d" % [segment_id, m, n]
raise ArgumentError,
"#{designator} can only occur #{limit.max} times"
"component element #{descriptor} cannot be further deconstructed"
end

unless value.node.children.defined_at?(n - 1)
return Either.failure("#{designator} occurs only #{value.node.children.length} times")
elsif element_use.repeatable?
# m: element of segment
# n: occurence of repeated element
# o: component of composite element
descriptor = "%s%02d" % [segment_id, m]
occurs_count = element_zip.children.count
unless element_use.repeat_count.include?(n)
raise ArgumentError,
"repeatable element #{descriptor} can only occur #{element_use.repeat_count.max} times"
end

value = value.child(n - 1)
n, o = o, nil

return Either.success(value) if n.nil?
end
if o.nil?
description = (element_use.composite?) ? "repeatable composite" : "repeatable"
if element_zip.node.blank?
return Either.failure("#{description} element #{descriptor} does not occur")
elsif occurs_count < n
return Either.failure("#{description} element #{descriptor} only occurs #{occurs_count} times")
else
return Either.success(element_zip.child(n - 1))
end

unless value.node.composite?
raise ArgumentError,
"#{designator} is a simple element"
end
elsif element_use.composite?
components = element_def.component_uses.length
unless o <= components
raise ArgumentError,
"repeatable composite element #{descriptor} only has #{components} components"
end

unless o.nil?
raise ArgumentError,
"#{designator} is a non-repeatable composite element"
end
descriptor = "%s%02d" % [segment_id, m]

unless n >= 1
raise ArgumentError,
"argument must be positive"
end
if element_zip.node.blank?
return Either.failure("repeatable composite element #{descriptor} does not occur")
elsif occurs_count < n
return Either.failure("repeatable composite element #{descriptor} only occurs #{occurs_count} times")
else
component_zip = element_zip.children.at(n - 1)
return Either.success(component_zip.child(o - 1))
end

length = definition.element_uses.at(m - 1).definition.component_uses.length
unless n <= length
raise ArgumentError,
"#{designator} has only #{length} components"
end
else
raise ArgumentError,
"repeatable element #{descriptor} cannot be further deconstructed"
end

if value.node.empty?
Either.failure("#{designator} is empty")
else
Either.success(value.child(n - 1))
raise ArgumentError,
"#{segment_id}#{"%02d" % m} is not a composite or repeated element"
end
end
end
Expand Down Expand Up @@ -514,6 +565,17 @@ def __find(invalid, id, elements, assert_repeatable = false)
state = zipper
value = zipper.node.zipper

if assert_repeatable
# We have to do some extra work here, because `target` below won't
# have the additional instructions that could be added by calling
# `.push` on the new state class
repeatable ||= begin
suppose = StateMachine.new(@config, [state])
suppose, = suppose.execute(op_, state, nil, filter_tok)
suppose.node.instructions.matches(filter_tok, true, :find).present?
end
end

# 1. Move upward (possibly zero times)
op_.pop_count.times do
value = value.up
Expand All @@ -524,7 +586,6 @@ def __find(invalid, id, elements, assert_repeatable = false)
# nodes to move left, but not exactly how many. Instead, we
# know what the InstructionTable is when we get there.
target = zipper.node.instructions.pop(op_.pop_count).drop(op_.drop_count)
repeatable ||= target.matches(filter_tok, true, :find).present?

# 3. If the segment we're searching for belongs in a new subtree,
# but it's not the only segment that might have "opened" that
Expand Down Expand Up @@ -626,12 +687,10 @@ def __find(invalid, id, elements, assert_repeatable = false)

if assert_repeatable and not repeatable
raise Exceptions::ParseError,
"#{id} segment is not repeatable"
end

if not reachable
"segment #{filter_tok.to_x12(Reader::Separators.default)} is not repeatable"
elsif not reachable
raise Exceptions::ParseError,
"segment #{filter_tok.to_x12(Reader::Separators.default)} cannot be reached from the current state"
"segment #{filter_tok.to_x12(Reader::Separators.default)} cannot be reached"
elsif matches.empty?
Either.failure("segment #{filter_tok.to_x12(Reader::Separators.default)} does not occur")
else
Expand Down
6 changes: 6 additions & 0 deletions lib/stupidedi/schema/code_list.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,12 @@ class << CodeList

# @return [CodeList::Internal]
def build(hash)
# @todo: deprecate
CodeList::Internal.new(hash)
end

# @return [CodeList::Internal]
def internal(hash)
CodeList::Internal.new(hash)
end

Expand Down
Loading

0 comments on commit 1235c80

Please sign in to comment.