[Specification::DSL] Fixes around Linter and Analyzer #233

mrackwitz · 2015-04-04T23:17:41Z

Includes a fix that :root_only => true attributes could been overwritten in subspecs.
Especially the change in a6d508f should been reviewed carefully.
/c @kylef @segiddins @alloy

segiddins · 2015-04-05T00:20:51Z

lib/cocoapods-core/specification/dsl.rb

@@ -76,8 +76,10 @@ module DSL
      #   @param  [String] name
      #           the name of the pod.
      #
-      root_attribute :name,
-                     :required => true
+      attribute :name,


name should definitely be a root attribute...

But it's also valid and required for subspecs.

ah but you shouldn't be able to set the subspecs name directly, it's set via spec.subspec 'ss_name' do |ss| bit

Agree on that, but if it's defined as root_attribute, it will be never validated for subspecs and the behavior, when the value is set, is undefined. It would be an exception among other root attributes, because it may occur on subspec level. I think being able to overwrite it in the subspec block in the DSL is the lesser of the two evils.

@mrackwitz in that case, we must validate that

if spec.parent spec.name.start_with?("#{spec.parent.name}/") end

@segiddins: Where would you add that check?

That's not required to lint Specification, because the names are not flattened. Subspecs store only the last name element in their attributes_hash[:name].

👍 in that case, need to validate that no / gets into attributes_hash[:name]

Yes, indeed. 👍

segiddins · 2015-04-06T19:56:36Z

CHANGELOG.md

@@ -2,6 +2,12 @@

 ## Master

+##### Enhancements
+
+* The linter fails now if root attributes occur on subspec level.


segiddins · 2015-04-06T20:00:26Z

This needs more testing around setting names in subspecs

mrackwitz · 2015-04-07T19:25:35Z

@segiddins: Added the linter validation for subspec names. Anything else, you see?

kylef · 2015-04-07T19:36:44Z

spec/specification/linter_spec.rb

@@ -145,6 +150,11 @@ def result_should_include(*values)
        result_should_include('name', 'whitespace')
      end

+      it 'fails a specification whose name contains a backslash' do
+        @spec.stubs(:name).returns('BananaKit/BananaFruit')


The test name says backslash but here we are testing a forward slash.

segiddins · 2015-04-07T19:44:15Z

@mrackwitz while you're at it, please make sure #177 and #178 are addressed?

segiddins · 2015-04-07T22:34:47Z

lib/cocoapods-core/specification/linter.rb

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


if spec.name

Not only root attributes.

mrackwitz · 2015-04-08T23:30:31Z

@segiddins: Addressed the issues.

kylef · 2015-04-09T09:27:46Z

CHANGELOG.md

@@ -9,6 +9,18 @@
  [Samuel Giddins](https://github.com/segiddins)
  [#221](https://github.com/CocoaPods/Core/issues/221)

+* The linter will now ensure that subspecs' names do not contain whitespace.  


IMO, this is a bug fix not an enhancement. (same goes for below entry)

Overall it will be likely rather perceived as bug fixes.

@segiddins

The spec is based off of the changes by @segiddins in #178 and @kaplin in #202. So this supersedes and closes #178, #202. This fixes #177.

[Specification::DSL] Fixes around Linter and Analyzer

mrackwitz added the s4:awaiting validation label Apr 4, 2015

segiddins reviewed Apr 5, 2015
View reviewed changes

segiddins mentioned this pull request Apr 6, 2015

[Specification] Allow specifying a custom module map file #218

Merged

2 tasks

mrackwitz force-pushed the mr-spec-dsl-fixes branch from 3e57238 to 1969089 Compare April 6, 2015 14:43

mrackwitz added a commit that referenced this pull request Apr 6, 2015

[CHANGELOG] Add entry for #233

1969089

segiddins reviewed Apr 6, 2015
View reviewed changes

mrackwitz force-pushed the mr-spec-dsl-fixes branch from 1969089 to ed08d80 Compare April 6, 2015 21:41

mrackwitz added a commit that referenced this pull request Apr 6, 2015

[CHANGELOG] Add entry for #233

ed08d80

kylef reviewed Apr 7, 2015
View reviewed changes

segiddins reviewed Apr 7, 2015
View reviewed changes

mrackwitz force-pushed the mr-spec-dsl-fixes branch from 4ee6737 to f0ce915 Compare April 8, 2015 10:17

mrackwitz added 9 commits April 8, 2015 12:34

[Doc] Fix wrong parameter type in Analyzer#validate_attribute_value

3149631

[Specification::DSL] root_attributes are not inherited

a08e014

[Specification::DSL] name isn't a root attribute

56f3521

[Specification::Linter] Check all required attributes

cd45dec

Not only root attributes.

[Specification::Linter] Fulfill spec stubs

ebce8de

[Specification::Analyzer] Forbid root attributes in subspecs

68e87bd

[Specification::DSL] Remove dead code from Attribute

85bc43d

[Doc] Everything is an Object

7fe4710

[Doc] Better description for Analyzer#validate_attribute_occurrence

cd2f623

mrackwitz force-pushed the mr-spec-dsl-fixes branch from 29ce6ad to 2df8f5f Compare April 8, 2015 10:36

mrackwitz added a commit that referenced this pull request Apr 8, 2015

[CHANGELOG] Add entry for #233

60a5a80

kylef reviewed Apr 9, 2015
View reviewed changes

mrackwitz added 5 commits April 9, 2015 11:31

[Specification::Linter] Disallow slashes in spec names

19400c0

[Specification::Linter] Extract spec root name validation in own method

b830026

[Specification::Linter] Forbid whitespaces, dots and slashes

0f80dba

The spec is based off of the changes by @segiddins in #178 and @kaplin in #202. So this supersedes and closes #178, #202. This fixes #177.

[CHANGELOG] Add entry for #233

7164b56

[CHANGELOG] Add entry for #177

4c4140a

mrackwitz force-pushed the mr-spec-dsl-fixes branch from 2df8f5f to 4c4140a Compare April 9, 2015 09:36

kylef added a commit that referenced this pull request Apr 9, 2015

Merge pull request #233 from CocoaPods/mr-spec-dsl-fixes

b609fa0

[Specification::DSL] Fixes around Linter and Analyzer

kylef merged commit b609fa0 into master Apr 9, 2015

mrackwitz deleted the mr-spec-dsl-fixes branch April 9, 2015 09:56

kylef removed the s4:awaiting validation label Apr 9, 2015

kylef restored the mr-spec-dsl-fixes branch April 12, 2015 04:48

kylef deleted the mr-spec-dsl-fixes branch April 12, 2015 04:51

mrackwitz added a commit that referenced this pull request Apr 12, 2015

[CHANGELOG] Add entry for #233

af295cb

mrackwitz added a commit that referenced this pull request Apr 12, 2015

[CHANGELOG] Add entry for #233

83a7984

Ashton-W pushed a commit to Ashton-W/Core that referenced this pull request Nov 2, 2015

[CHANGELOG] Add entry for CocoaPods#233

1fdf048

Ashton-W pushed a commit to Ashton-W/Core that referenced this pull request Nov 2, 2015

Merge pull request CocoaPods#233 from CocoaPods/mr-spec-dsl-fixes

f1dd21a

[Specification::DSL] Fixes around Linter and Analyzer

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Specification::DSL] Fixes around Linter and Analyzer #233

[Specification::DSL] Fixes around Linter and Analyzer #233

mrackwitz commented Apr 4, 2015

segiddins Apr 5, 2015

mrackwitz Apr 5, 2015

segiddins Apr 5, 2015

mrackwitz Apr 6, 2015

segiddins Apr 6, 2015

mrackwitz Apr 6, 2015

segiddins Apr 6, 2015

mrackwitz Apr 6, 2015

segiddins Apr 6, 2015

mrackwitz Apr 6, 2015

segiddins Apr 6, 2015

segiddins commented Apr 6, 2015

mrackwitz commented Apr 7, 2015

kylef Apr 7, 2015

segiddins commented Apr 7, 2015

segiddins Apr 7, 2015

mrackwitz commented Apr 8, 2015

kylef Apr 9, 2015

mrackwitz Apr 9, 2015

[Specification::DSL] Fixes around Linter and Analyzer #233

[Specification::DSL] Fixes around Linter and Analyzer #233

Conversation

mrackwitz commented Apr 4, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

segiddins commented Apr 6, 2015

mrackwitz commented Apr 7, 2015

Choose a reason for hiding this comment

segiddins commented Apr 7, 2015

Choose a reason for hiding this comment

mrackwitz commented Apr 8, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment