Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions spec/frontmatter_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
require 'spec_helper'

RSpec.describe '.md files' do
Dir["#{REPO_ROOT}/content/**/*.md"].sort.each do |path|
page = path.split(REPO_ROOT.to_s).last

describe page do
it 'does not include duplicate keys in YAML frontmatter' do
_, frontmatter, _content = File.read(path).split('---', 3)

expect(YAMLHelper.duplicate_keys(frontmatter)).to be_empty
end
end
end
end
1 change: 1 addition & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
config.disable_monkey_patching!
end

REPO_ROOT = Pathname.new(File.expand_path('../..', __FILE__))
SITE_ROOT = Pathname.new(File.expand_path('../../_site', __FILE__))

def file_at(path)
Expand Down
35 changes: 35 additions & 0 deletions spec/support/yaml_helper.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
module YAMLHelper
module_function

# from https://stackoverflow.com/a/55705853
def duplicate_keys(file_or_content)
yaml = file_or_content.is_a?(File) ? file_or_content.read : file_or_content
duplicate_keys = []

validator = ->(node, parent_path) do
if node.is_a?(Psych::Nodes::Mapping)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add psych to Gemfile so that we're not relying on a transitive dependency?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The built-in ruby parser is psych these days, so I think it's fine as-is?

irb(main):001:0> defined?(Psych)
=> nil
irb(main):002:0> require 'yaml'
=> true
irb(main):003:0> defined?(Psych)
=> "constant"

Copy link
Copy Markdown
Contributor

@aduth aduth Apr 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The built-in ruby parser is psych these days, so I think it's fine as-is?

irb(main):001:0> defined?(Psych)
=> nil
irb(main):002:0> require 'yaml'
=> true
irb(main):003:0> defined?(Psych)
=> "constant"

Ah, I didn't know that, was primarily going by...

https://github.com/18F/identity-idp/blob/fca67c6ddb41b3768abc346eddf89314f70c8916/Gemfile#L110

Seems fine.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW the other place we use it isn't the IDP, it's the idp-config repo which doesn't have psych in its Gemfile either

children = node.children.each_slice(2) # In a Mapping, every other child is the key node, the other is the value node.
duplicates = children.map { |key_node, _value_node| key_node }.group_by(&:value).select { |_value, nodes| nodes.size > 1 }

duplicates.each do |key, nodes|
duplicate_key = {
file: (file_or_content.path if file_or_content.is_a?(File)),
key: parent_path + [key],
occurrences: nodes.map { |occurrence| "line: #{occurrence.start_line + 1}" },
}.compact

duplicate_keys << duplicate_key
end

children.each { |key_node, value_node| validator.call(value_node, parent_path + [key_node&.value].compact) }
else
node.children.to_a.each { |child| validator.call(child, parent_path) }
end
end

ast = Psych.parse_stream(yaml)
validator.call(ast, [])

duplicate_keys
end
end