Skip to content

Commit

Permalink
Merge pull request CocoaPods#122 from CocoaPods/better_linter_msgs
Browse files Browse the repository at this point in the history
Add labels to all the linter warnings and errors
  • Loading branch information
kapin committed May 2, 2014
2 parents 355ad5c + 311f4fb commit 03491ae
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 53 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
78 changes: 45 additions & 33 deletions lib/cocoapods-core/specification/linter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -173,74 +174,76 @@ 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

# Performs validations related to the `summary` attribute.
#
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

# Performs validations related to the `description` attribute.
#
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

# Performs validations related to the `homepage` attribute.
#
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

# Performs validations related to the `frameworks` attribute.
#
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

# Performs validations related to the `weak frameworks` attribute.
#
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

Expand All @@ -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
Expand All @@ -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

Expand All @@ -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

Expand All @@ -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
Expand All @@ -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

Expand All @@ -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

Expand Down
27 changes: 16 additions & 11 deletions lib/cocoapods-core/specification/linter/analyzer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand All @@ -68,23 +69,27 @@ 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

# Check the hooks
#
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
Expand Down
5 changes: 5 additions & 0 deletions spec/specification/linter/analyzer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
18 changes: 9 additions & 9 deletions spec/specification/linter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 03491ae

Please sign in to comment.