Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add labels to all the linter warnings and errors #122

Merged
merged 2 commits into from
May 2, 2014
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
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