Skip to content

Commit

Permalink
Merge pull request #233 from CocoaPods/mr-spec-dsl-fixes
Browse files Browse the repository at this point in the history 
[Specification::DSL] Fixes around Linter and Analyzer
  • Loading branch information
@kylef
kylef committed Apr 9, 2015
2 parents 7dcd237 + 4c4140a commit b609fa0
Show file tree
Hide file tree
Showing 9 changed files with 156 additions and 142 deletions.
14 changes: 13 additions & 1 deletion CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,21 @@
[Samuel Giddins](https://github.com/segiddins)
[#221](https://github.com/CocoaPods/Core/issues/221)


##### Bug Fixes

* The linter will now ensure that subspecs' names do not contain whitespace.
[Marius Rackwitz](https://github.com/mrackwitz)
[Joshua Kalpin](https://github.com/Kapin)
[Samuel Giddins](https://github.com/segiddins)
[#177](https://github.com/CocoaPods/Core/issues/177)
[#178](https://github.com/CocoaPods/Core/pull/178)
[#202](https://github.com/CocoaPods/Core/pull/202)
[#233](https://github.com/CocoaPods/Core/pull/233)

* The linter fails now if root attributes occur on subspec level.
[Marius Rackwitz](https://github.com/mrackwitz)
[#233](https://github.com/CocoaPods/Core/pull/233)

* Inhibit warnings for pods that only have the `inhibit_warnings` option enabled
on a subspec.
[Samuel Giddins](https://github.com/segiddins)
Expand Down
4 changes: 4 additions & 0 deletions lib/cocoapods-core/specification/consumer.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ def self.spec_attr_accessor(name)

# @!group Regular attributes

# @return [String] The name of the specification.
#
spec_attr_accessor :name

# @return [Bool] Whether the source files of the specification require to
# be compiled with ARC.
#
Expand Down
9 changes: 5 additions & 4 deletions lib/cocoapods-core/specification/dsl.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,10 @@ module DSL
# @param [String] name
# the name of the pod.
#
root_attribute :name,
:required => true
attribute :name,
:required => true,
:inherited => false,
:multi_platform => false

#------------------#

Expand Down Expand Up @@ -794,8 +796,7 @@ def dependency(*args)
# @param [String] name
# the module name.
#
root_attribute :module_name,
:inherited => true
root_attribute :module_name

#------------------#

Expand Down
58 changes: 0 additions & 58 deletions lib/cocoapods-core/specification/dsl/attribute.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,65 +185,7 @@ def writer_name
def writer_singular_form
"#{name.to_s.singularize}=" if singularize?
end

#---------------------------------------------------------------------#

# @!group Values validation

# Validates the value for an attribute. This validation should be
# performed before the value is prepared or wrapped.
#
# @note The this is called before preparing the value.
#
# @raise If the type is not in the allowed list.
#
# @return [void]
#
def validate_type(value)
return if value.nil?
unless supported_types.any? { |klass| value.class == klass }
raise StandardError, "Non acceptable type `#{value.class}` for "\
"#{self}. Allowed values: `#{types.inspect}`"
end
end

# Validates a value before storing.
#
# @raise If a root only attribute is set in a subspec.
#
# @raise If a unknown key is added to a hash.
#
# @return [void]
#
def validate_for_writing(spec, value)
if root_only? && !spec.root?
raise StandardError, "Can't set `#{name}` attribute for " \
"subspecs (in `#{spec.name}`)."
end

if keys
value.keys.each do |key|
unless allowed_keys.include?(key)
raise StandardError, "Unknown key `#{key}` for "\
"#{self}. Allowed keys: `#{allowed_keys.inspect}`"
end
end
end

# @return [Array] the flattened list of the allowed keys for the
# hash of a given specification.
#
def allowed_keys
if keys.is_a?(Hash)
keys.keys.concat(keys.values.flatten.compact)
else
keys
end
end
end
end

#-----------------------------------------------------------------------#
end
end
end
65 changes: 38 additions & 27 deletions lib/cocoapods-core/specification/linter.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ def initialize(spec_or_path)
def lint
@results = Results.new
if spec
check_required_root_attributes
validate_root_name
check_required_attributes
run_root_validation_hooks
perform_all_specs_analysis
else
Expand Down Expand Up @@ -80,15 +81,32 @@ def warnings

# !@group Lint steps

# Checks that every root only attribute which is required has a value.
# Checks that the spec's root name matches the filename.
#
# @return [void]
#
def check_required_root_attributes
attributes = DSL.attributes.values.select(&:root_only?)
def validate_root_name
if spec.root.name && file
acceptable_names = [
spec.root.name + '.podspec',
spec.root.name + '.podspec.json',
]
names_match = acceptable_names.include?(file.basename.to_s)
unless names_match
results.add_error('name', 'The name of the spec should match the ' \
'name of the file.')
end
end
end

# Checks that every required attribute has a value.
#
# @return [void]
#
def check_required_attributes
attributes = DSL.attributes.values.select(&:required?)
attributes.each do |attr|
value = spec.send(attr.name)
next unless attr.required?
unless value && (!value.respond_to?(:empty?) || !value.empty?)
if attr.name == :license
results.add_warning('attributes', 'Missing required attribute ' \
Expand Down Expand Up @@ -146,7 +164,7 @@ def run_all_specs_validation_hooks
#
# @note Hooks are called only if there is a value for the attribute as
# required attributes are already checked by the
# {#check_required_root_attributes} step.
# {#check_required_attributes} step.
#
# @return [void]
#
Expand All @@ -164,34 +182,27 @@ def run_validation_hooks(attributes, target)

private

# @!group Root spec validation helpers

# Performs validations related to the `name` attribute.
#
def _validate_name(_n)
if spec.name && file
acceptable_names = [
spec.root.name + '.podspec',
spec.root.name + '.podspec.json',
]
names_match = acceptable_names.include?(file.basename.to_s)
unless names_match
results.add_error('name', 'The name of the spec should match the ' \
'name of the file.')
end
def _validate_name(name)
if name =~ /\//
results.add_error('name', 'The name of a spec should not contain ' \
'a slash.')
end

if spec.root.name =~ /\s/
results.add_error('name', 'The name of a spec should not contain ' \
'whitespace.')
end
if name =~ /\s/
results.add_error('name', 'The name of a spec should not contain ' \
'whitespace.')
end

if spec.root.name[0, 1] == '.'
results.add_error('name', 'The name of a spec should not begin' \
' with a period.')
end
if name[0, 1] == '.'
results.add_error('name', 'The name of a spec should not begin' \
' with a period.')
end
end

# @!group Root spec validation helpers

def _validate_authors(a)
if a.is_a? Hash
if a == { 'YOUR NAME HERE' => 'YOUR EMAIL HERE' }
Expand Down
29 changes: 27 additions & 2 deletions lib/cocoapods-core/specification/linter/analyzer.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@ def check_attributes
end

Pod::Specification::DSL.attributes.each do |_key, attribute|
validate_attribute_type(attribute, consumer.spec.attributes_hash[attribute.name.to_s])
declared_value = consumer.spec.attributes_hash[attribute.name.to_s]
validate_attribute_occurrence(attribute, declared_value)
validate_attribute_type(attribute, declared_value)
if attribute.name != :platforms
value = value_for_attribute(attribute)
validate_attribute_value(attribute, value) if value
Expand Down Expand Up @@ -106,6 +108,14 @@ def check_if_spec_is_empty

private

# Returns the own or inherited (if applicable) value of the
# given attribute.
#
# @param [Spec::DSL::Attribute] attribute
# The attribute.
#
# @return [mixed]
#
def value_for_attribute(attribute)
if attribute.root_only?
consumer.spec.send(attribute.name)
Expand All @@ -117,12 +127,27 @@ def value_for_attribute(attribute)
nil
end

# Validates that root attributes don't occur in subspecs.
#
# @param [Spec::DSL::Attribute] attribute
# The attribute.

# @param [Object] value
# The value of the attribute.
#
def validate_attribute_occurrence(attribute, value)
if attribute.root_only? && !value.nil? && !consumer.spec.root?
results.add_error('attributes', "Can't set `#{attribute.name}` attribute for " \
"subspecs (in `#{consumer.spec.name}`).")
end
end

# Validates the given value for the given attribute.
#
# @param [Spec::DSL::Attribute] attribute
# The attribute.
#
# @param [Spec::DSL::Attribute] value
# @param [Object] value
# The value of the attribute.
#
def validate_attribute_value(attribute, value)
Expand Down
40 changes: 0 additions & 40 deletions spec/specification/dsl/attribute_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,45 +113,5 @@ module Pod
attr.writer_singular_form.should == 'framework='
end
end

#-------------------------------------------------------------------------#

describe 'Writer method support' do
it 'validates a value to check whether it is compatible with the accepted types' do
attr = Attribute.new(:frameworks, :types => [String], :container => Array)
lambda { attr.validate_type('a string') }.should.not.raise
lambda { attr.validate_type(['with container']) }.should.not.raise
lambda { attr.validate_type(:non_accepted) }.should.raise StandardError
end

it 'validates root only values before writing' do
attr = Attribute.new(:summary, :root_only => true)
spec = Spec.new do |s|
s.subspec 'sub' do |_sp|
end
end
subspec = spec.subspecs.first

lambda { attr.validate_for_writing(spec, 'a string') }.should.not.raise
lambda { attr.validate_for_writing(subspec, 'a string') }.should.raise StandardError
end

it 'validates the allowed keys for hashes before writing' do
attr = Attribute.new(:source, :keys => [:git])
spec = Spec.new
lambda { attr.validate_for_writing(spec, :git => 'repo') }.should.not.raise
lambda { attr.validate_for_writing(spec, :snail_mail => 'repo') }.should.raise StandardError
end

it 'returns the allowed keys' do
attr = Attribute.new(:source, :keys => [:git, :svn])
attr.allowed_keys.should == [:git, :svn]
end

it 'returns the allowed keys flattening keys specified in a hash' do
attr = Attribute.new(:source, :keys => { :git => [:tag, :commit], :http => nil })
attr.allowed_keys.map(&:to_s).sort.should == %w(commit git http tag)
end
end
end
end
17 changes: 17 additions & 0 deletions spec/specification/linter/analyzer_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,23 @@ module Pod

#----------------------------------------#

describe 'Root attributes' do
it 'fails a subspec with a root attribute' do
subspec = @spec.subspec 'subspec' do |sp|
sp.homepage = 'http://example.org'
end
results = Specification::Linter::Results.new
@analyzer = Specification::Linter::Analyzer.new(subspec.consumer(:ios), results)
results = @analyzer.analyze
results.count.should.be.equal(1)
expected = 'Can\'t set `homepage` attribute for subspecs (in `BananaLib/subspec`).'
results.first.message.should.include?(expected)
results.first.attribute_name.should.include?('attribute')
end
end

#----------------------------------------#

describe 'Unknown keys check' do
it 'validates a spec with valid keys' do
results = @analyzer.analyze
Expand Down
Loading

0 comments on commit b609fa0

Please sign in to comment.