From 06a6a08ae614afab3c253239452de5049166e7a9 Mon Sep 17 00:00:00 2001 From: Joshua Kalpin Date: Sun, 27 Apr 2014 20:37:40 -0400 Subject: [PATCH 1/2] Add labels to all the linter warnings and errors Every warning and error should now begin with [CATEGORY]. Updated a bunch of tests as well to check that these are included. --- lib/cocoapods-core/specification/linter.rb | 78 +++++++++++-------- .../specification/linter/analyzer.rb | 27 ++++--- spec/specification/linter/analyzer_spec.rb | 5 ++ spec/specification/linter_spec.rb | 18 ++--- 4 files changed, 75 insertions(+), 53 deletions(-) diff --git a/lib/cocoapods-core/specification/linter.rb b/lib/cocoapods-core/specification/linter.rb index 2d9797a01..ed7583dd4 100644 --- a/lib/cocoapods-core/specification/linter.rb +++ b/lib/cocoapods-core/specification/linter.rb @@ -50,7 +50,8 @@ def lint run_root_validation_hooks perform_all_specs_analysis else - error "The specification defined in `#{file}` could not be loaded." \ + error "[spec] The specification defined in `#{file}` could not be" \ + ' loaded.' \ "\n\n#{@raise_message}" end results.empty? @@ -91,9 +92,9 @@ def check_required_root_attributes next unless attr.required? unless value && (!value.respond_to?(:empty?) || !value.empty?) if attr.name == :license - warning("Missing required attribute `#{attr.name}`.") + warning("[attributes] Missing required attribute `#{attr.name}`.") else - error("Missing required attribute `#{attr.name}`.") + error("[attributes] Missing required attribute `#{attr.name}`.") end end end @@ -173,24 +174,25 @@ def _validate_name(n) ] names_match = acceptable_names.include?(file.basename.to_s) unless names_match - error 'The name of the spec should match the name of the file.' + error '[name] The name of the spec should match the name of the \ + file.' end if spec.root.name =~ /\s/ - error 'The name of a spec should not contain whitespace.' + error '[name] The name of a spec should not contain whitespace.' end if spec.root.name[0, 1] == '.' - error 'The name of a spec should not begin with a period.' + error '[name] The name of a spec should not begin with a period.' end end end def _validate_version(v) if v.to_s.empty? - error 'A version is required.' + error '[version] A version is required.' elsif v <= Version::ZERO - error 'The version of the spec should be higher than 0.' + error '[version] The version of the spec should be higher than 0.' end end @@ -198,11 +200,11 @@ def _validate_version(v) # def _validate_summary(s) if s.length > 140 - warning "The summary should be a short version of `description` " \ - "(max 140 characters)." + warning '[summary] The summary should be a short version of' \ + '`description` (max 140 characters).' end if s =~ /A short description of/ - warning 'The summary is not meaningful.' + warning '[summary] The summary is not meaningful.' end end @@ -210,13 +212,13 @@ def _validate_summary(s) # def _validate_description(d) if d =~ /An optional longer description of/ - warning 'The description is not meaningful.' + warning '[description] The description is not meaningful.' end if d == spec.summary - warning 'The description is equal to the summary.' + warning '[description] The description is equal to the summary.' end if d.length < spec.summary.length - warning 'The description is shorter than the summary.' + warning '[description] The description is shorter than the summary.' end end @@ -224,7 +226,7 @@ def _validate_description(d) # def _validate_homepage(h) if h =~ %r{http://EXAMPLE} - warning 'The homepage has not been updated from default' + warning '[homepage] The homepage has not been updated from default' end end @@ -232,7 +234,7 @@ def _validate_homepage(h) # def _validate_frameworks(frameworks) if frameworks_invalid?(frameworks) - error 'A framework should only be specified by its name' + error '[frameworks] A framework should only be specified by its name' end end @@ -240,7 +242,8 @@ def _validate_frameworks(frameworks) # def _validate_weak_frameworks(frameworks) if frameworks_invalid?(frameworks) - error 'A weak framework should only be specified by its name' + error '[weak_frameworks] A weak framework should only be specified' \ + 'by its name' end end @@ -250,15 +253,18 @@ def _validate_libraries(libs) libs.each do |lib| lib = lib.downcase if lib.end_with?('.a') || lib.end_with?('.dylib') - error "Libraries should not include the extension (`#{lib}`)" + error '[libraries] Libraries should not include the extension ' \ + "(`#{lib}`)" end if lib.start_with?('lib') - error "Libraries should omit the `lib` prefix (`#{lib}`)" + error '[libraries] Libraries should omit the `lib` prefix' \ + " (`#{lib}`)" end if lib.include?(',') - error "Libraries should not include comas (`#{lib}`)" + error '[libraries] Libraries should not include comas' \ + "(`#{lib}`)" end end end @@ -268,13 +274,13 @@ def _validate_libraries(libs) def _validate_license(l) type = l[:type] if type.nil? - warning 'Missing license type.' + warning '[license] Missing license type.' end if type && type.gsub(' ', '').gsub("\n", '').empty? - warning 'Invalid license type.' + warning '[license] Invalid license type.' end if type && type =~ /\(example\)/ - error 'Sample license type.' + error '[license] Sample license type.' end end @@ -286,20 +292,21 @@ def _validate_source(s) version = spec.version.to_s if git =~ %r{http://EXAMPLE} - error 'The Git source still contains the example URL.' + error '[source] The Git source still contains the example URL.' end if commit && commit.downcase =~ /head/ - error 'The commit of a Git source cannot be `HEAD`.' + error '[source] The commit of a Git source cannot be `HEAD`.' end if tag && !tag.to_s.include?(version) - warning 'The version should be included in the Git tag.' + warning '[source] The version should be included in the Git tag.' end if version == '0.0.1' if commit.nil? && tag.nil? - error 'Git sources should specify either a commit or a tag.' + error '[source] Git sources should specify either a commit or' \ + 'a tag.' end else - warning 'Git sources should specify a tag.' if tag.nil? + warning '[source] Git sources should specify a tag.' if tag.nil? end end @@ -315,14 +322,17 @@ def perform_github_source_checks(s) return unless git =~ /^#{URI.regexp}$/ git_uri = URI.parse(git) if git_uri.host == 'www.github.com' - warning 'Github repositories should not use `www` in URL.' + warning '[github_sources] Github repositories should not use' \ + '`www` in URL.' end if git_uri.host == 'github.com' || git_uri.host == 'gist.github.com' unless git.end_with?('.git') - warning 'Github repositories should end in `.git`.' + warning '[github_sources] Github repositories should end in' \ + '`.git`.' end unless git_uri.scheme == 'https' - warning 'Github repositories should use `https` link.' + warning '[github_sources] Github repositories should use' \ + '`https` link.' end end end @@ -332,7 +342,8 @@ def perform_github_source_checks(s) # def _validate_social_media_url(s) if s =~ %r{https://twitter.com/EXAMPLE} - warning 'The social media URL has not been updated from default' + warning '[social_media_url] The social media URL has not been' \ + 'updated from default' end end @@ -346,7 +357,8 @@ def _validate_social_media_url(s) # def _validate_compiler_flags(flags) if flags.join(' ').split(' ').any? { |flag| flag.start_with?('-Wno') } - warning "Warnings must not be disabled (`-Wno' compiler flags)." + warning '[compiler_flags] Warnings must not be disabled' \ + "(`-Wno compiler' flags)." end end diff --git a/lib/cocoapods-core/specification/linter/analyzer.rb b/lib/cocoapods-core/specification/linter/analyzer.rb index 3f0e0c1df..1f9736a1e 100644 --- a/lib/cocoapods-core/specification/linter/analyzer.rb +++ b/lib/cocoapods-core/specification/linter/analyzer.rb @@ -35,8 +35,8 @@ def validate_file_patterns end patterns.each do |pattern| if pattern.start_with?('/') - error "File patterns must be relative and cannot start with a " \ - "slash (#{attrb.name})." + error '[File Patterns] File patterns must be relative ' \ + "and cannot start with a slash (#{attrb.name})." end end end @@ -55,8 +55,9 @@ def check_tmp_arc_not_nil end unless declared - warning "A value for `requires_arc` should be specified until the " \ - "migration to a `true` default." + warning '[requires_arc] A value for `requires_arc` should be' \ + ' specified until the ' \ + 'migration to a `true` default.' end end @@ -68,9 +69,11 @@ def check_if_spec_is_empty empty_patterns = methods.all? { |m| consumer.send(m).empty? } empty = empty_patterns && consumer.spec.subspecs.empty? if empty - error "The #{consumer.spec} spec is empty (no source files, " \ - "resources, resource_bundles, preserve paths, vendored_libraries, " \ - "vendored_frameworks dependencies or subspecs)." + error "[File Patterns] The #{consumer.spec} spec is empty" + '(no source files, ' \ + 'resources, resource_bundles, preserve paths,' \ + 'vendored_libraries, vendored_frameworks dependencies' \ + 'or subspecs).' end end @@ -78,13 +81,15 @@ def check_if_spec_is_empty # def check_install_hooks unless consumer.spec.pre_install_callback.nil? - warning "The pre install hook has been deprecated, " \ - "use the `resource_bundles` or the `prepare_command` attributes." + warning '[pre_install_hook] The pre install hook has been' \ + ' deprecated, ' \ + 'use the `resource_bundles` or the `prepare_command` attributes.' end unless consumer.spec.post_install_callback.nil? - warning "The post install hook has been deprecated, " \ - "use the `resource_bundles` or the `prepare_command` attributes." + warning '[post_install_hook] The post install hook has been' \ + ' deprecated, ' \ + 'use the `resource_bundles` or the `prepare_command` attributes.' end end end diff --git a/spec/specification/linter/analyzer_spec.rb b/spec/specification/linter/analyzer_spec.rb index 899937507..af83cc4f8 100644 --- a/spec/specification/linter/analyzer_spec.rb +++ b/spec/specification/linter/analyzer_spec.rb @@ -21,6 +21,7 @@ module Pod @subject.results.count.should.be.equal(1) expected = 'patterns must be relative' @subject.results.first.message.should.include?(expected) + @subject.results.first.message.should.include?('File Patterns') end it 'checks if a specification is empty' do @@ -36,6 +37,7 @@ module Pod @subject.analyze @subject.results.count.should.be.equal(1) @subject.results.first.message.should.include?('spec is empty') + @subject.results.first.message.should.include?('File Patterns') end end @@ -48,6 +50,7 @@ module Pod @subject.results.count.should.be.equal(1) expected = '`requires_arc` should be specified' @subject.results.first.message.should.include?(expected) + @subject.results.first.message.should.include?('requires_arc') end it 'supports the declaration of the attribute per platform' do @@ -78,6 +81,7 @@ module Pod @subject.results.count.should.be.equal(1) expected = 'pre install hook has been deprecated' @subject.results.first.message.should.include?(expected) + @subject.results.first.message.should.include?('pre_install_hook') end it 'checks if the post install hook has been defined' do @@ -86,6 +90,7 @@ module Pod @subject.results.count.should.be.equal(1) expected = 'post install hook has been deprecated' @subject.results.first.message.should.include?(expected) + @subject.results.first.message.should.include?('post_install_hook') end end end diff --git a/spec/specification/linter_spec.rb b/spec/specification/linter_spec.rb index 71fe44927..8999b7856 100644 --- a/spec/specification/linter_spec.rb +++ b/spec/specification/linter_spec.rb @@ -345,33 +345,33 @@ def message_should_include(*values) it 'checks that libraries do not end with a .a extension' do @spec.libraries = %w(z.a) - message_should_include('should not include the extension', 'z.a') + message_should_include('should not include the extension', 'z.a', + 'libraries') end it 'checks that libraries do not end with a .dylib extension' do @spec.libraries = %w(ssl.dylib) - message_should_include('should not include the extension', 'ssl.dylib') + message_should_include('should not include the extension', 'ssl.dylib', + 'libraries') end it 'checks that libraries do not begin with lib' do @spec.libraries = %w(libz) - message_should_include('should omit the `lib` prefix', 'libz') + message_should_include('should omit the `lib` prefix', 'libz', + 'libraries') end it 'checks that libraries do not contain unwanted characters' do @spec.libraries = ['ssl, z'] - message_should_include('should not include comas', 'ssl, z') + message_should_include('should not include comas', 'ssl, z', + 'libraries') end #------------------# it 'checks if the compiler flags disable warnings' do @spec.compiler_flags = '-some_flag', '-another -Wno_flags' - message_should_include('warnings', 'disabled') - @linter.lint - message = @linter.results.first.message - message.should.include('Warnings') - message.should.include('disabled') + message_should_include('warnings', 'disabled', 'compiler_flags') end end end From e6fedba3da429c1b4190da7c340b09e515c44e4f Mon Sep 17 00:00:00 2001 From: Joshua Kalpin Date: Fri, 2 May 2014 11:29:32 -0400 Subject: [PATCH 2/2] Changelog and fixing a quote --- CHANGELOG.md | 5 +++++ lib/cocoapods-core/specification/linter.rb | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e2487dc9..9e695377d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,11 @@ [#115](https://github.com/CocoaPods/Core/issues/115) [#116](https://github.com/CocoaPods/Core/pull/116) +* Linter warnings and errors are now prefixed with \[`ATTRIBUTE_NAME`\]. + This `ATTRIBUTE_NAME` specifies which property caused the error/warning. + [Joshua Kalpin][Kapin] + [#50](https://github.com/CocoaPods/Core/pull/122) + ## 0.32.1 ## 0.32.0 diff --git a/lib/cocoapods-core/specification/linter.rb b/lib/cocoapods-core/specification/linter.rb index ed7583dd4..a094c3ddd 100644 --- a/lib/cocoapods-core/specification/linter.rb +++ b/lib/cocoapods-core/specification/linter.rb @@ -358,7 +358,7 @@ def _validate_social_media_url(s) def _validate_compiler_flags(flags) if flags.join(' ').split(' ').any? { |flag| flag.start_with?('-Wno') } warning '[compiler_flags] Warnings must not be disabled' \ - "(`-Wno compiler' flags)." + '(`-Wno compiler` flags).' end end