Skip to content

Commit

Permalink
resolves asciidoctor#2244 warn if duplicate ID is detected
Browse files Browse the repository at this point in the history
  • Loading branch information
mojavelinux committed Jul 11, 2017
1 parent 2a7d41d commit 950dcda
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 14 deletions.
31 changes: 19 additions & 12 deletions lib/asciidoctor/parser.rb
Expand Up @@ -459,7 +459,7 @@ def self.next_block(reader, parent, attributes = {}, options = {})

# QUESTION should we introduce a parsing context object?
source_location = reader.cursor if sourcemap
this_line = reader.read_line
this_path, this_lineno, this_line = reader.path, reader.lineno, reader.read_line
delimited_block = block_context = cloaked_context = terminator = nil
style = attributes[1] ? (parse_style_attribute attributes, reader) : nil

Expand All @@ -477,7 +477,7 @@ def self.next_block(reader, parent, attributes = {}, options = {})
elsif block_extensions && extensions.registered_for_block?(style, block_context)
block_context = style.to_sym
else
warn %(asciidoctor: WARNING: #{reader.prev_line_info}: invalid style for #{block_context} block: #{style})
warn %(asciidoctor: WARNING: #{this_path}: line #{this_lineno}: invalid style for #{block_context} block: #{style})
style = block_context.to_s
end
end
Expand Down Expand Up @@ -608,16 +608,15 @@ def self.next_block(reader, parent, attributes = {}, options = {})
unless match[1] == expected_index.to_s
warn %(asciidoctor: WARNING: #{reader.path}: line #{list_item_lineno}: callout list item index: expected #{expected_index} got #{match[1]})
end
list_item = next_list_item(reader, block, match)
expected_index += 1
if list_item
if (list_item = next_list_item reader, block, match)
block << list_item
if (coids = document.callouts.callout_ids block.items.size).empty?
warn %(asciidoctor: WARNING: #{reader.path}: line #{list_item_lineno}: no callouts refer to list item #{block.items.size})
else
list_item.attributes['coids'] = coids
end
end
expected_index += 1
match = nil
end

Expand Down Expand Up @@ -690,7 +689,7 @@ def self.next_block(reader, parent, attributes = {}, options = {})
# advance to block parsing =>
break
else
warn %(asciidoctor: WARNING: #{reader.prev_line_info}: invalid style for paragraph: #{style})
warn %(asciidoctor: WARNING: #{this_path}: line #{this_lineno}: invalid style for paragraph: #{style})
style = nil
# continue to process paragraph
end
Expand Down Expand Up @@ -899,7 +898,7 @@ def self.next_block(reader, parent, attributes = {}, options = {})
# REVIEW we may no longer need this nil check
# FIXME we've got to clean this up, it's horrible!
if block
block.source_location = source_location if source_location
block.source_location = source_location if sourcemap
# FIXME title should be assigned when block is constructed
block.title = attributes.delete 'title' if attributes.key? 'title'
#unless attributes.key? 'reftext'
Expand All @@ -909,7 +908,9 @@ def self.next_block(reader, parent, attributes = {}, options = {})
#block.style = attributes.delete 'style'
block.style = attributes['style']
if (block_id = (block.id ||= attributes['id']))
document.register :refs, [block_id, block, attributes['reftext'] || (block.title? ? block.title : nil)]
unless document.register :refs, [block_id, block, attributes['reftext'] || (block.title? ? block.title : nil)]
warn %(asciidoctor: WARNING: #{this_path}: line #{this_lineno}: id assigned to block already in use: #{block_id})
end
end
# FIXME remove the need for this update!
block.attributes.update(attributes) unless attributes.empty?
Expand Down Expand Up @@ -1207,7 +1208,9 @@ def self.catalog_inline_anchors text, block, document
next if (reftext.include? '{') && (reftext = document.sub_attributes reftext).empty?
end
end
document.register :refs, [id, (Inline.new block, :anchor, reftext, :type => :ref, :id => id), reftext]
unless document.register :refs, [id, (Inline.new block, :anchor, reftext, :type => :ref, :id => id), reftext]
warn %(asciidoctor: WARNING: #{document.reader.path}: id assigned to anchor already in use: #{id})
end
end if (text.include? '[[') || (text.include? 'or:')
nil
end
Expand All @@ -1222,7 +1225,9 @@ def self.catalog_inline_anchors text, block, document
def self.catalog_inline_biblio_anchor text, block, document
if InlineBiblioAnchorRx =~ text
# QUESTION should we sub attributes in reftext (like with regular anchors)?
document.register :refs, [(id = $1), (Inline.new block, :anchor, (reftext = %([#{$2 || id}])), :type => :bibref, :id => id), reftext]
unless document.register :refs, [(id = $1), (Inline.new block, :anchor, (reftext = %([#{$2 || id}])), :type => :bibref, :id => id), reftext]
warn %(asciidoctor: WARNING: #{document.reader.path}: id assigned to bibliography anchor already in use: #{id})
end
end
nil
end
Expand Down Expand Up @@ -1558,7 +1563,7 @@ def self.read_lines_for_list_item(reader, list_type, sibling_trait = nil, has_te
def self.initialize_section reader, parent, attributes = {}
document = parent.document
source_location = reader.cursor if document.sourcemap
sect_id, sect_reftext, sect_title, sect_level, _ = parse_section_title reader, document
sect_id, sect_reftext, sect_title, sect_level, single_line = parse_section_title reader, document
if sect_reftext
attributes['reftext'] = sect_reftext
elsif attributes.key? 'reftext'
Expand Down Expand Up @@ -1605,7 +1610,9 @@ def self.initialize_section reader, parent, attributes = {}
# generate an ID if one was not embedded or specified as anchor above section title
if (id = section.id ||= (attributes['id'] ||
((document.attributes.key? 'sectids') ? (Section.generate_id section.title, document) : nil)))
document.register :refs, [id, section, sect_reftext || section.title]
unless document.register :refs, [id, section, sect_reftext || section.title]
warn %(asciidoctor: WARNING: #{reader.path}: line #{reader.lineno - (single_line ? 1 : 2)}: id assigned to section already in use: #{id})
end
end

section.update_attributes(attributes)
Expand Down
6 changes: 4 additions & 2 deletions test/sections_test.rb
Expand Up @@ -219,10 +219,11 @@
content
EOS

doc = document_from_string input
doc, warnings = redirect_streams {|_, err| [(document_from_string input), err.string]}
reftext = doc.catalog[:ids]['install']
refute_nil reftext
assert_equal 'First Install', reftext
assert_includes warnings, 'line 7: id assigned to section already in use: install'
end

test 'duplicate block id should not overwrite existing section id entry in references table' do
Expand All @@ -236,10 +237,11 @@
content
EOS

doc = document_from_string input
doc, warnings = redirect_streams {|_, err| [(document_from_string input), err.string] }
reftext = doc.catalog[:ids]['install']
refute_nil reftext
assert_equal 'First Install', reftext
assert_includes warnings, 'line 7: id assigned to block already in use: install'
end
end

Expand Down

0 comments on commit 950dcda

Please sign in to comment.