diff --git a/.cirrus.yml b/.cirrus.yml index 91b4d9999b..ed7b308a63 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -87,7 +87,7 @@ validate_metadata_task: - ./ci/validate_metadata.sh validate_ci_tests_task: - skip: "!changesInclude('ci_tests/*', 'ci/*')" + skip: "!changesInclude('ci_tests/**', 'ci/**')" eks_container: <<: *CONTAINER_DEFINITION dockerfile: ci/Dockerfile diff --git a/ci/asciidoc_validation/custom-asciidoctor b/ci/asciidoc_validation/custom-asciidoctor index 86b33baded..0bf43109a3 100755 --- a/ci/asciidoc_validation/custom-asciidoctor +++ b/ci/asciidoc_validation/custom-asciidoctor @@ -1,18 +1,24 @@ -#!/usr/bin/ruby +#!/usr/bin/env ruby # Based on asciidoctor main ruby script. # This is only meant to introspect and log which asciidoc files were used. require 'asciidoctor' require 'asciidoctor/cli' +MAIN_FILE_REGEX = /^.*\/rules\/(?S\d+)\/(?:(?\w+)\/)?rule.adoc$/ + class MainFileLogger < Asciidoctor::Extensions::Preprocessor include Asciidoctor::Logging def process document, reader + # Enable sourcemap to track source location. + # This is useful to report more accurate errors in other loggers. + document.sourcemap = true + main_file = document.normalize_system_path(reader.file, document.reader.dir) # This assumes unix-style path separator. - if nil == main_file.match(/^.*\/rules\/S\d+\/(\w+\/)?rule.adoc$/) + if nil == main_file.match(MAIN_FILE_REGEX) abort("Main file does not follow expected pattern: #{main_file}") end @@ -73,9 +79,87 @@ class IncludeLogger < Asciidoctor::Extensions::IncludeProcessor # Intentionnaly no process function here. end +class SourceLogger < Asciidoctor::Extensions::TreeProcessor + include Asciidoctor::Logging + + def get_source_location block + loc = block.source_location # Asciidoctor::Reader::Cursor. + return "#{loc.file}:#{loc.lineno}" + end + + def get_rule document + main_file = document.normalize_system_path(document.reader.file, document.reader.dir) + res = main_file.match(MAIN_FILE_REGEX) + lang = res[:lang] || 'default' + "#{res[:id]}/#{lang}" + end + + def process document + rule = get_rule(document) + source_blocks = document.find_by(context: :listing, style: 'source') + + # Collect individually valid blocks per diff-id. + blocks_per_id = Hash.new { |hash, key| hash[key] = Array.new } + + # Find blocks with only diff-id but no diff-type, or vice-versa, or invalid diff-type. + source_blocks.each do |block| + id = block.attr('diff-id', nil) + type = block.attr('diff-type', nil) + + if !id and !type + next # Nothing to validate + end + + loc = get_source_location(block) + + if !id + logger.info("ASCIIDOC LOGGER DIFF:[#{rule}] diff-id is missing in #{loc}") + next + end + + if !type + logger.info("ASCIIDOC LOGGER DIFF:[#{rule}] diff-type is missing in #{loc}") + next + elsif !['compliant', 'noncompliant'].include?(type) + logger.info("ASCIIDOC LOGGER DIFF:[#{rule}] diff-type '#{type}' is not valid in #{loc}") + next + end + + # The block is valid on its own. + blocks_per_id[id].push(block) + end + + # Each diff-id should have exactly 1 compliant and 1 noncompliant block. + # Find blocks that break this rule. + blocks_per_id.each do |id, blocks| + # Sort to ensure deterministic output. + blocks.sort_by! { |block| get_source_location(block) } + + case blocks.length + when 1 + loc = get_source_location(blocks[0]) + logger.info("ASCIIDOC LOGGER DIFF:[#{rule}] Incomplete example for diff-id=#{id}, missing counterpart for #{loc}") + when 2 + type1 = blocks[0].attr('diff-type') + type2 = blocks[1].attr('diff-type') + if type1 == type2 + loc1 = get_source_location(blocks[0]) + loc2 = get_source_location(blocks[1]) + logger.info("ASCIIDOC LOGGER DIFF:[#{rule}] Two #{type1} examples for diff-id=#{id}: #{loc1} and #{loc2}") + end + else + locs = blocks.map { |block| get_source_location(block) } + locs = locs.join(', ') + logger.info("ASCIIDOC LOGGER DIFF:[#{rule}] #{blocks.length} examples for diff-id=#{id}: #{locs}") + end + end + end +end + Asciidoctor::Extensions.register do preprocessor MainFileLogger include_processor IncludeLogger.new @document + treeprocessor SourceLogger end invoker = Asciidoctor::Cli::Invoker.new ARGV diff --git a/ci/asciidoc_validation/validate.sh b/ci/asciidoc_validation/validate.sh index 4dce35be05..67f296833a 100755 --- a/ci/asciidoc_validation/validate.sh +++ b/ci/asciidoc_validation/validate.sh @@ -22,6 +22,7 @@ # asciidoctor: INFO: ASCIIDOC LOGGER MAIN FILE: $PATH # asciidoctor: INFO: ASCIIDOC LOGGER INCLUDED: $PATH # asciidoctor: INFO: ASCIIDOC LOGGER CROSSREFERENCE: $RULEID cross-references $PATH +# asciidoctor: INFO: ASCIIDOC LOGGER DIFF: $VALIDATION_FAILURE_MESSAGE set -euo pipefail @@ -39,21 +40,19 @@ grep_nofail() { grep "$@" || [ "$?" == "1" ] } +extract_messages_from_log() { + # The first 3 columns of the log are not relevant. + # The 4th (and any that follows if ':' is used in the message) + # provides the relevant validation error message. + cut -d ':' -f 4- | sort -u +} + find "${RULES_DIR}" -name 'rule.adoc' \ | xargs "${SCRIPT_DIR}/custom-asciidoctor" -a rspecator-view --verbose -R "${RULES_DIR}" -D "${TMPOUT_DIR}" 2>&1 \ | grep_nofail -e 'ASCIIDOC LOGGER' \ > "${TMPOUT_DIR}/asciidoc_introspection" -grep -ve 'CROSSREFERENCE' "${TMPOUT_DIR}/asciidoc_introspection" \ - | cut -d ':' -f 4 \ - | sort -u \ - > "${TMPOUT_DIR}/used_asciidoc_files" - -git ls-files --cached -- "${RULES_DIR}/**.adoc" "${SHARED_CONTENT_DIR}/**.adoc" \ - | xargs realpath \ - > "${TMPOUT_DIR}/all_asciidoc_files" - -cross_references=$(grep_nofail -e 'CROSSREFERENCE' "${TMPOUT_DIR}/asciidoc_introspection" | cut -d ':' -f 4 | sort -u) +cross_references=$(grep_nofail -e 'CROSSREFERENCE' "${TMPOUT_DIR}/asciidoc_introspection" | extract_messages_from_log) if [[ -n "$cross_references" ]]; then echo >&2 'ERROR: Some rules try to include content from unallowed directories.' echo >&2 'To share content between rules, you should use the "shared_content" folder at the root of the repository.' @@ -62,10 +61,29 @@ if [[ -n "$cross_references" ]]; then exit_code=1 fi +grep_nofail -Pe '(INCLUDE|MAIN FILE)' "${TMPOUT_DIR}/asciidoc_introspection" \ + | extract_messages_from_log \ + > "${TMPOUT_DIR}/used_asciidoc_files" + +git ls-files --cached -- "${RULES_DIR}/**.adoc" "${SHARED_CONTENT_DIR}/**.adoc" \ + | xargs realpath \ + > "${TMPOUT_DIR}/all_asciidoc_files" + orphans=$(comm -1 -3 <(sort -u "${TMPOUT_DIR}/used_asciidoc_files") <(sort -u "${TMPOUT_DIR}/all_asciidoc_files")) -if [[ -n "$orphans" ]]; then +if [[ -n "$orphans" ]] +then printf >&2 'ERROR: These adoc files are not included anywhere:\n-----\n%s\n-----\n' "$orphans" exit_code=1 fi +bad_diffs=$(grep_nofail -e 'DIFF' "${TMPOUT_DIR}/asciidoc_introspection" | extract_messages_from_log) +if [[ -n "$bad_diffs" ]] +then + printf >&2 'ERROR: Diff highlighting is used incorrectly:\n-----\n%s\n-----\n' "$bad_diffs" + if [[ -n "${RUNNING_CI_INTEGRATION_TEST:-}" ]] + then + exit_code=1 # FIXME: there are currently validation errors in the repo. Unconditionally enable this line when they are all fixed. + fi +fi + exit $exit_code diff --git a/ci_tests/asciidoc_validation/run_tests.sh b/ci_tests/asciidoc_validation/run_tests.sh index 670ea4b2c7..f8f254f5b2 100755 --- a/ci_tests/asciidoc_validation/run_tests.sh +++ b/ci_tests/asciidoc_validation/run_tests.sh @@ -4,6 +4,8 @@ set -uo pipefail +export RUNNING_CI_INTEGRATION_TEST=1 # FIXME remove this once it's no longer needed in validate.sh + # We could write complex checks to ensure only specific commands fail and emit # a specific error message. Instead, we rely on `set -xe` to consistently and # reliably exit with non-zero if any command fails and pinpoint which command @@ -77,4 +79,22 @@ run_test "test_bad_cross_ref" \ "S100 cross-references .*rules/S1000/bad.adoc" \ "S1000 cross-references .*rules/S100/java/bad.adoc" +run_test "test_diff_source" \ + "ERROR: Diff highlighting is used incorrectly:" \ + "\[S100/cfamily] diff-type is missing in .*/rules/S100/cfamily/rule.adoc:3" \ + "\[S100/cfamily] diff-id is missing in .*/rules/S100/cfamily/rule.adoc:8" \ + "\[S100/cfamily] diff-type 'bad' is not valid in .*/rules/S100/cfamily/rule.adoc:13" \ + "\[S100/cfamily] diff-type is missing in .*/rules/S100/cfamily/local.adoc:3" \ + "\[S100/cfamily] diff-id is missing in .*/rules/S100/cfamily/local.adoc:8" \ + "\[S100/cfamily] diff-type 'local' is not valid in .*/rules/S100/cfamily/local.adoc:13" \ + "\[S100/cfamily] diff-type is missing in .*/shared_content/cfamily/shared.adoc:3" \ + "\[S100/cfamily] diff-id is missing in .*/shared_content/cfamily/shared.adoc:8" \ + "\[S100/cfamily] diff-type 'shared' is not valid in .*/shared_content/cfamily/shared.adoc:13" \ + "\[S100/java] Incomplete example for diff-id=1, missing counterpart for .*/rules/S100/java/rule.adoc:3" \ + "\[S100/java] Incomplete example for diff-id=2, missing counterpart for .*/rules/S100/java/rule.adoc:8" \ + "\[S100/java] Incomplete example for diff-id=3, missing counterpart for .*/shared_content/java/example.adoc:3" \ + "\[S100/java] 3 examples for diff-id=4: .*/rules/S100/java/rule.adoc:15, .*/shared_content/java/example.adoc:13, .*/shared_content/java/example.adoc:8" \ + "\[S200/default] 3 examples for diff-id=1: .*/rules/S200/rule.adoc:12, .*/rules/S200/rule.adoc:2, .*/rules/S200/rule.adoc:7" \ + "\[S200/default] Two noncompliant examples for diff-id=2: .*/rules/S200/rule.adoc:17 and .*/rules/S200/rule.adoc:22" + echo "All tests passed" diff --git a/ci_tests/asciidoc_validation/test_diff_source/rules/S100/cfamily/local.adoc b/ci_tests/asciidoc_validation/test_diff_source/rules/S100/cfamily/local.adoc new file mode 100644 index 0000000000..51ee76074d --- /dev/null +++ b/ci_tests/asciidoc_validation/test_diff_source/rules/S100/cfamily/local.adoc @@ -0,0 +1,15 @@ + +[source,cpp,diff-id=1] +---- +Missing diff-type +---- + +[source,cpp,diff-type=compliant] +---- +Missing diff-id +---- + +[source,c,diff-id=1,diff-type=local] +---- +Bad diff-type +---- \ No newline at end of file diff --git a/ci_tests/asciidoc_validation/test_diff_source/rules/S100/cfamily/rule.adoc b/ci_tests/asciidoc_validation/test_diff_source/rules/S100/cfamily/rule.adoc new file mode 100644 index 0000000000..4d59101be8 --- /dev/null +++ b/ci_tests/asciidoc_validation/test_diff_source/rules/S100/cfamily/rule.adoc @@ -0,0 +1,19 @@ + +[source,cpp,diff-id=1] +---- +Missing diff-type +---- + +[source,cpp,diff-type=compliant] +---- +Missing diff-id +---- + +[source,c,diff-id=1,diff-type=bad] +---- +Bad diff-type +---- + +include::./local.adoc[] + +include::../../../shared_content/cfamily/shared.adoc[] diff --git a/ci_tests/asciidoc_validation/test_diff_source/rules/S100/java/rule.adoc b/ci_tests/asciidoc_validation/test_diff_source/rules/S100/java/rule.adoc new file mode 100644 index 0000000000..24438f48be --- /dev/null +++ b/ci_tests/asciidoc_validation/test_diff_source/rules/S100/java/rule.adoc @@ -0,0 +1,17 @@ + +[source,java,diff-id=1,diff-type=compliant] +---- +1. compliant but missing noncompliant +---- + +[source,java,diff-id=2,diff-type=noncompliant] +---- +2. noncompliant but missing compliant +---- + +include::../../../shared_content/java/example.adoc[] + +[source,java,diff-id=4,diff-type=noncompliant] +---- +4. noncompliant B +---- diff --git a/ci_tests/asciidoc_validation/test_diff_source/rules/S200/rule.adoc b/ci_tests/asciidoc_validation/test_diff_source/rules/S200/rule.adoc new file mode 100644 index 0000000000..d6346a750d --- /dev/null +++ b/ci_tests/asciidoc_validation/test_diff_source/rules/S200/rule.adoc @@ -0,0 +1,24 @@ +[source,diff-id=1,diff-type=compliant] +---- +1. compliant +---- + +[source,diff-id=1,diff-type=noncompliant] +---- +1. noncompliant A +---- + +[source,diff-id=1,diff-type=noncompliant] +---- +1. noncompliant B +---- + +[source,diff-id=2,diff-type=noncompliant] +---- +2. noncompliant A +---- + +[source,diff-id=2,diff-type=noncompliant] +---- +2. noncompliant B +---- diff --git a/ci_tests/asciidoc_validation/test_diff_source/shared_content/cfamily/shared.adoc b/ci_tests/asciidoc_validation/test_diff_source/shared_content/cfamily/shared.adoc new file mode 100644 index 0000000000..05f9067512 --- /dev/null +++ b/ci_tests/asciidoc_validation/test_diff_source/shared_content/cfamily/shared.adoc @@ -0,0 +1,15 @@ + +[source,cpp,diff-id=1] +---- +Missing diff-type +---- + +[source,cpp,diff-type=compliant] +---- +Missing diff-id +---- + +[source,c,diff-id=1,diff-type=shared] +---- +Bad diff-type +---- \ No newline at end of file diff --git a/ci_tests/asciidoc_validation/test_diff_source/shared_content/java/example.adoc b/ci_tests/asciidoc_validation/test_diff_source/shared_content/java/example.adoc new file mode 100644 index 0000000000..c13bc8de04 --- /dev/null +++ b/ci_tests/asciidoc_validation/test_diff_source/shared_content/java/example.adoc @@ -0,0 +1,15 @@ + +[source,java,diff-id=3,diff-type=compliant] +---- +3. compliant but missing noncompliant +---- + +[source,java,diff-id=4,diff-type=compliant] +---- +4. compliant but too many noncompliant +---- + +[source,java,diff-id=4,diff-type=noncompliant] +---- +4. noncompliant A +---- diff --git a/ci_tests/asciidoc_validation/test_valid/rules/S100/cfamily/rule.adoc b/ci_tests/asciidoc_validation/test_valid/rules/S100/cfamily/rule.adoc index 8d96c4307f..5889328303 100644 --- a/ci_tests/asciidoc_validation/test_valid/rules/S100/cfamily/rule.adoc +++ b/ci_tests/asciidoc_validation/test_valid/rules/S100/cfamily/rule.adoc @@ -3,3 +3,13 @@ S100 cfamily include::./another.adoc[] include::../../../shared_content/anything.adoc[] + +[source,cpp,diff-id=1,diff-type=noncompliant] +---- +noncompliant +---- + +[source,cpp,diff-id=1,diff-type=compliant] +---- +compliant +----