From 215539481075da9363ccd00bafb9653ba05fe83a Mon Sep 17 00:00:00 2001 From: j2blake Date: Fri, 19 Jul 2013 16:26:33 -0400 Subject: [PATCH] VIVO-137 improve i18nChecker Recognize sets of language-dependent template files. Only report warnings if they use i18n() Reduce the number of false positives Speed things up. --- utilities/i18nChecker/i18nChecker.rb | 6 + .../i18nChecker/template_file_checker.rb | 12 +- utilities/i18nChecker/template_set_checker.rb | 174 ++++++++++++++++++ 3 files changed, 186 insertions(+), 6 deletions(-) create mode 100644 utilities/i18nChecker/template_set_checker.rb diff --git a/utilities/i18nChecker/i18nChecker.rb b/utilities/i18nChecker/i18nChecker.rb index 39b14c4a..147883e5 100644 --- a/utilities/i18nChecker/i18nChecker.rb +++ b/utilities/i18nChecker/i18nChecker.rb @@ -33,6 +33,7 @@ and optional "complete" or "summary" flags. E.g.: $: << File.dirname(File.expand_path(__FILE__)) require 'properties_file_checker' require 'template_file_checker' +require 'template_set_checker' class I18nChecker # ------------------------------------------------------------------------------------ @@ -101,6 +102,11 @@ class I18nChecker end def process_template_files(paths, summary) + ts = TemplateSetChecker.new(paths) + ts.report(summary) + paths = ts.remaining_paths + @total_warnings += ts.warnings_count + paths.each() do |path| tf = TemplateFileChecker.new(path) tf.report(summary) diff --git a/utilities/i18nChecker/template_file_checker.rb b/utilities/i18nChecker/template_file_checker.rb index 45c6bfe9..6e4b1230 100644 --- a/utilities/i18nChecker/template_file_checker.rb +++ b/utilities/i18nChecker/template_file_checker.rb @@ -117,23 +117,23 @@ class TemplateFileChecker end def scan_for_words_outside_of_tags() - scan(/>\s*([^><]+)\s*\s*([^><]+)]*title=(["'])\s*([^>]+?)\s*\1.*?>/mi, 2, "Words found in title attribute of an HTML tag") + scan(/<[^>]*title=(["'])\s*([^>]*?)\s*\1.*?>/mi, 2, "Words found in title attribute of an HTML tag") end def scan_for_alert_attributes() - scan(/]*alert=(["'])\s*([^>]+?)\s*\1.*?>/mi, 2, "Words found in alert attribute of tag") + scan(/]*alert=(["'])\s*([^>]*?)\s*\1.*?>/mi, 2, "Words found in alert attribute of tag") end def scan_for_alt_attributes() - scan(/]*alt=(["'])\s*([^>]+?)\s*\1.*?>/mi, 2, "Words found in alt attribute of tag") + scan(/]*alt=(["'])\s*([^>]*?)\s*\1.*?>/mi, 2, "Words found in alt attribute of tag") end def scan_for_value_attributes_on_submit_tags() - scan(/]*type=["']submit["'][^>]*value=(["'])\s*([^'">]+?)\s*\1.*?>/mi, 2, "Words found in value attribute of tag") + scan(/]*type=["']submit["'][^>]*value=(["'])\s*([^'">]*?)\s*\1.*?>/mi, 2, "Words found in value attribute of tag") end # ------------------------------------------------------------------------------------ @@ -157,7 +157,6 @@ class TemplateFileChecker def report(summary) puts " Template file '#{@path}'" - if !@warnings.empty? if summary puts " #{@warnings.size} warnings." @@ -168,6 +167,7 @@ class TemplateFileChecker end end end + $stdout.flush end def warnings() diff --git a/utilities/i18nChecker/template_set_checker.rb b/utilities/i18nChecker/template_set_checker.rb new file mode 100644 index 00000000..967daa04 --- /dev/null +++ b/utilities/i18nChecker/template_set_checker.rb @@ -0,0 +1,174 @@ +=begin +-------------------------------------------------------------------------------- + +Look at a set of template paths, and remove any paths that represent +language-specific templates. Warn if any of those templates are using the i18n() +Freemarker method. + +Note: any template path that ends in _xx.ftl or _xx_YY.ftl is assumed to be +language-specific. As a heuristic, we will also assume that any template with +the same path, but without the language-specifier, is also language-specific, +representing the default version of the template. + +-------------------------------------------------------------------------------- +=end + +class Warning + attr_reader :line + attr_reader :message + + def initialize(line, message) + @line = line + @message = message + end +end + +class TemplateSet + attr_reader :root + attr_reader :paths + attr_reader :warnings + + def initialize(root) + @root = root + @paths = [] + @warnings = [] + end + + def add_path(path) + @paths << path + end + + def add_warning(warning) + @warnings << warning + end +end + +class TemplateSetChecker + # ------------------------------------------------------------------------------------ + private + # ------------------------------------------------------------------------------------ + + def find_template_sets() + find_language_specific_templates() + find_default_language_templates() + remove_from_remaining_paths() + end + + def find_language_specific_templates() + @paths.each() do |path| + if is_language_specific(path) + add_to_template_sets(path) + end + end + end + + def is_language_specific(path) + /_[a-z]{2}(_[A-Z]{2})?\.ftl$/ =~ path + end + + def add_to_template_sets(path) + root = root_path(path) + set = @template_sets[root] || TemplateSet.new(root) + set.add_path(path) + @template_sets[root] = set + end + + def root_path(path) + if /(.*?)(_[a-z]{2}(_[A-Z]{2})?)?\.ftl$/ =~ path + $1 + else + path + end + end + + def find_default_language_templates() + @paths.each() do |path| + root = root_path(path) + set = @template_sets[root] + if set + set.add_path(path) unless set.paths.include?(path) + end + end + end + + def remove_from_remaining_paths() + @remaining_paths = Array.new(@paths) + @template_sets.each_value do |set| + set.paths.each do |path| + @remaining_paths.delete(path) + end + end + end + + def scan_set(set) + set.paths.each do |path| + load(path) + @contents.gsub(/i18n/) do |s| + offset = $~.begin(0) + set.add_warning(Warning.new(line_number(offset), "in #{path}: Call to i18n() in a language-specific template.")) + s + end + end + end + + def load(path) + @line_offsets = [] + IO.open(File.new(path).fileno) do |f| + @contents = f.read() + pos = 0 + while found = @contents.index(/\n/, pos) + pos = found + 1 + @line_offsets << pos + end + end + end + + def line_number(offset) + return @line_offsets.find_index() {|o| o > offset} || @line_offsets.size + end + + + # ------------------------------------------------------------------------------------ + public + # ------------------------------------------------------------------------------------ + + def initialize(paths) + @paths = paths + @template_sets = {} + @remaining_paths = [] + + find_template_sets() + @template_sets.each_value do |set| + scan_set(set) + end + end + + def report(summary) + @template_sets.each_value do |set| + puts" Template file set '#{set.paths.join("'\n '")}'" + if !set.warnings.empty? + if summary + puts " #{set.warnings.size} warnings." + else + set.warnings.sort! {|a, b| a.line <=> b.line} + set.warnings.each do |w| + puts " line #{w.line}: #{w.message}" + end + end + end + end + $stdout.flush + end + + def warnings_count() + count = 0 + @template_sets.each_value do |set| + count += set.warnings.size + end + count + end + + def remaining_paths() + @remaining_paths + end +end