[Analyzer] Move scoping logic into PodVariantSet

Which brings proper scoping for libraries and frameworks as well.

lib/cocoapods/installer/analyzer.rb

@@ -14,6 +14,7 @@ module Pod
       autoload :SpecsState,                'cocoapods/installer/analyzer/specs_state'
       autoload :LockingDependencyAnalyzer, 'cocoapods/installer/analyzer/locking_dependency_analyzer'
       autoload :PodVariant,                'cocoapods/installer/analyzer/pod_variant'
+      autoload :PodVariantSet,             'cocoapods/installer/analyzer/pod_variant_set'
       autoload :TargetInspectionResult,    'cocoapods/installer/analyzer/target_inspection_result'
       autoload :TargetInspector,           'cocoapods/installer/analyzer/target_inspector'
 
@@ -298,19 +299,16 @@ module Pod
           end
 
           pod_targets = distinct_targets.flat_map do |_root, target_definitions_by_variant|
-            # Frameworks and libraries can safely co-exist
-            target_definitions_by_variant.group_by { |k, _| k.requires_frameworks? }.flat_map do |_, array|
-              if array.count > 1
-                # There are different sets of subspecs or the spec is used across different platforms
-                hash = Hash[array]
-                suffixes = scope_suffixes_for_variants(hash.keys)
-                hash.flat_map do |variant, target_definitions|
-                  generate_pod_target(target_definitions, variant.specs, :scope_suffix => suffixes[variant])
-                end
-              else
-                variant, target_definitions = *array.first
-                generate_pod_target(target_definitions, variant.specs)
+            if target_definitions_by_variant.count > 1
+              # There are different sets of subspecs or the spec is used across different platforms
+              hash = Hash[array]
+              suffixes = PodVariantSet.new(hash.keys).scope_suffixes
+              hash.flat_map do |variant, target_definitions|
+                generate_pod_target(target_definitions, variant.specs, :scope_suffix => suffixes[variant])
               end
+            else
+              variant, target_definitions = *array.first
+              generate_pod_target(target_definitions, variant.specs)
             end
           end
         else
@@ -327,49 +325,6 @@ module Pod
         end
       end
 
-      # Describes what makes pod targets configurations distinct among other.
-      #
-      # @param [Array<PodVariant>] variants
-      #
-      # @return [Hash<PodVariant, String>]
-      #
-      def scope_suffixes_for_variants(variants)
-        all_spec_variants = variants.map(&:specs)
-        all_platform_variants = variants.map(&:platform)
-        all_platform_name_variants = all_platform_variants.map(&:name)
-
-        if all_platform_name_variants.uniq.count == all_platform_name_variants.count
-          # => Platform name
-          platform_name_proc = proc { |platform| Platform.string_name(platform.symbolic_name).tr(' ', '') }
-        else
-          # => Platform name + SDK version
-          platform_name_proc = proc { |platform| platform.to_s.tr(' ', '') }
-        end
-
-        if all_platform_variants.uniq.count == all_platform_variants.count
-          result = variants.map { |v| [v, platform_name_proc.call(v.platform)] }
-        else
-          common_specs = all_spec_variants.reduce(all_spec_variants.first, &:&)
-          result = variants.map do |variant|
-            subspecs = variant.specs - common_specs
-            subspec_names = subspecs.map do |spec|
-              spec.root? ? 'root' : spec.name.split('/')[1..-1].join('_')
-            end.sort
-            # => Subspecs names without common subspecs
-            [variant, subspec_names.empty? ? nil : subspec_names.join('-')]
-          end
-          if all_spec_variants.count > all_spec_variants.uniq.count
-            result.map! do |variant, suffix|
-              platform_name = platform_name_proc.call(variant.platform)
-              # => Platform name (+ SDK version) + subspecs names without common subspecs
-              [variant, [platform_name, suffix].compact.join('-')]
-            end
-          end
-        end
-
-        Hash[result.map { |v, name| [v, name && name.tr('/', '_')] }]
-      end
-
       # Finds the names of the Pods upon which the given target _transitively_
       # depends.
       #

lib/cocoapods/installer/analyzer/pod_variant_set.rb

+module Pod
+  class Installer
+    class Analyzer
+      # Collects all {PodVariant}.
+      class PodVariantSet
+        # @return [Array<PodVariant>] the different variants.
+        #
+        attr_accessor :variants
+
+        # @param [Array<PodVariant>] variants  @see #variants
+        #
+        def initialize(variants)
+          self.variants = variants
+        end
+
+        # Describes what makes each {PodVariant} distinct among the others.
+        #
+        # @return [Hash<PodVariant, String>]
+        #
+        def scope_suffixes
+          return { variants.first => nil } if variants.count == 1
+          scope_by_build_type
+        end
+
+        # Groups the collection by result of the block.
+        #
+        # @param [Block<Variant, #hash>] block
+        # @return [Array<PodVariantSet>]
+        #
+        def group_by(&block)
+          variants.group_by(&block).map { |_, v| PodVariantSet.new(v) }
+        end
+
+        # @private
+        #
+        # Prepends the given scoped {PodVariant}s with another scoping label, if there
+        # was more than one group of {PodVariant}s given.
+        #
+        # @param [Array<Hash<PodVariant, String>>] scoped_variants
+        #        {PodVariant}s, which where grouped on base of a criteria, which is used
+        #        in the block argument to generate a descriptive label.
+        #
+        # @param [Block<PodVariant, String>] block
+        #        takes a {PodVariant} and returns a scope suffix which is prepended, if
+        #        necessary.
+        #
+        # @return [Hash<PodVariant, String>]
+        #
+        def scope_if_necessary(scoped_variants, &block)
+          if scoped_variants.count == 1
+            return scoped_variants.first
+          end
+          Hash[scoped_variants.flat_map do |variants|
+            variants.map do |variant, suffix|
+              prefix = block.call(variant)
+              scope = [prefix, suffix].compact.join('-')
+              [variant, !scope.empty? ? scope : nil]
+            end
+          end]
+        end
+
+        # @private
+        # @return [Hash<PodVariant, String>]
+        #
+        def scope_by_build_type
+          scope_if_necessary(group_by(&:requires_frameworks).map(&:scope_by_platform)) do |variant|
+            variant.requires_frameworks? ? 'framework' : 'library'
+          end
+        end
+
+        # @private
+        # @return [Hash<PodVariant, String>]
+        #
+        def scope_by_platform
+          grouped_variants = group_by { |v| v.platform.name }
+          if grouped_variants.all? { |set| set.variants.count == 1 }
+            # => Platform name
+            platform_name_proc = proc { |v| Platform.string_name(v.platform.symbolic_name).tr(' ', '') }
+          else
+            grouped_variants = group_by(&:platform)
+            # => Platform name + SDK version
+            platform_name_proc = proc { |v| v.platform.to_s.tr(' ', '') }
+          end
+          scope_if_necessary(grouped_variants.map(&:scope_by_specs), &platform_name_proc)
+        end
+
+        # @private
+        # @return [Hash<PodVariant, String>]
+        #
+        def scope_by_specs
+          grouped_variants = group_by(&:specs)
+          all_spec_variants = grouped_variants.map { |set| set.variants.first.specs }
+          common_specs = all_spec_variants.reduce(all_spec_variants.first, &:&)
+          scope_if_necessary(grouped_variants.map(&:scope_without_suffix)) do |variant|
+            subspecs = variant.specs - common_specs
+            subspec_names = subspecs.map do |spec|
+              spec.root? ? 'root' : spec.name.split('/')[1..-1].join('_')
+            end.sort
+            subspec_names.empty? ? nil : subspec_names.join('-')
+          end
+        end
+
+        # @private
+        #
+        # Helps to define scope suffixes recursively.
+        #
+        # @return [Hash<PodVariant, String>]
+        #
+        def scope_without_suffix
+          Hash[variants.map { |v| [v, nil] }]
+        end
+      end
+    end
+  end
+end

spec/unit/installer/analyzer/pod_variant_set_spec.rb

+require File.expand_path('../../../../spec_helper', __FILE__)
+
+module Pod
+  describe PodVariantSet = Installer::Analyzer::PodVariantSet do
+    describe '#scope_suffixes' do
+      before do
+        @root_spec = stub(:name => 'Spec', :root? => true)
+      end
+
+      PodVariant = Pod::Installer::Analyzer::PodVariant.freeze
+
+      it 'returns scopes by built types if they qualify' do
+        variants = PodVariantSet.new([
+          PodVariant.new([@root_spec], Platform.ios, true),
+          PodVariant.new([@root_spec], Platform.ios, false),
+        ])
+        variants.scope_suffixes.values.should == %w(framework library)
+      end
+
+      it 'returns scopes by platform names if they qualify' do
+        variants = PodVariantSet.new([
+          PodVariant.new([@root_spec], Platform.ios),
+          PodVariant.new([@root_spec], Platform.osx),
+        ])
+        variants.scope_suffixes.values.should == %w(iOS OSX)
+      end
+
+      it 'returns scopes by versioned platform names if they qualify' do
+        variants = PodVariantSet.new([
+          PodVariant.new([@root_spec], Platform.ios),
+          PodVariant.new([@root_spec], Platform.new(:ios, '7.0')),
+        ])
+        variants.scope_suffixes.values.should == ['iOS', 'iOS7.0']
+      end
+
+      it 'returns scopes by subspec names if they qualify' do
+        shared_subspec = stub(:name => 'Spec/Shared', :root? => false)
+        variants = PodVariantSet.new([
+          PodVariant.new([@root_spec, shared_subspec], Platform.ios),
+          PodVariant.new([@root_spec, shared_subspec, stub(:name => 'Spec/Foo', :root? => false)], Platform.ios),
+          PodVariant.new([@root_spec, shared_subspec, stub(:name => 'Spec/Bar', :root? => false)], Platform.ios),
+        ])
+        variants.scope_suffixes.values.should == [nil, 'Foo', 'Bar']
+      end
+
+      it 'returns scopes by subspec names if they qualify and handle partial root spec presence well' do
+        variants = PodVariantSet.new([
+          PodVariant.new([stub(:name => 'Spec/Foo', :root? => false)], Platform.ios),
+          PodVariant.new([@root_spec, stub(:name => 'Spec/Bar', :root? => false)], Platform.ios),
+        ])
+        variants.scope_suffixes.values.should == ['Foo', 'Bar-root']
+      end
+
+      it 'returns scopes by platform names and subspec names if they qualify' do
+        variants = PodVariantSet.new([
+          PodVariant.new([@root_spec], Platform.ios),
+          PodVariant.new([@root_spec, stub(:name => 'Spec/Foo', :root? => false)], Platform.ios),
+          PodVariant.new([@root_spec], Platform.osx),
+          PodVariant.new([@root_spec, stub(:name => 'Spec/Bar', :root? => false)], Platform.osx),
+        ])
+        variants.scope_suffixes.values.should == [
+          'iOS',
+          'iOS-Foo',
+          'OSX',
+          'OSX-Bar',
+        ]
+      end
+
+      it 'returns scopes by versioned platform names and subspec names if they qualify' do
+        variants = PodVariantSet.new([
+          PodVariant.new([@root_spec], Platform.new(:ios, '7.0')),
+          PodVariant.new([@root_spec, stub(:name => 'Spec/Foo', :root? => false)], Platform.ios),
+          PodVariant.new([@root_spec], Platform.osx),
+          PodVariant.new([@root_spec, stub(:name => 'Spec/Bar', :root? => false)], Platform.osx),
+        ])
+        variants.scope_suffixes.values.should == [
+          'iOS7.0',
+          'iOS',
+          'OSX',
+          'OSX-Bar',
+        ]
+      end
+
+      it 'returns scopes by built types, versioned platform names and subspec names' do
+        variants = PodVariantSet.new([
+          PodVariant.new([@root_spec], Platform.new(:ios, '7.0')),
+          PodVariant.new([@root_spec], Platform.ios),
+          PodVariant.new([@root_spec], Platform.osx, true),
+          PodVariant.new([@root_spec, stub(:name => 'Spec/Foo', :root? => false)], Platform.osx, true),
+        ])
+        variants.scope_suffixes.values.should == [
+          'library-iOS7.0',
+          'library-iOS',
+          'framework',
+          'framework-Foo',
+        ]
+      end
+    end
+  end
+end

spec/unit/installer/analyzer_spec.rb

@@ -130,81 +130,6 @@ module Pod
 
       #--------------------------------------#
 
-      describe '#scope_suffixes_for_variants' do
-        before do
-          @analyzer = Pod::Installer::Analyzer.new(config.sandbox, stub('Podfile'), nil)
-          @root_spec = stub(:name => 'Spec', :root? => true)
-        end
-
-        PodVariant = Pod::Installer::Analyzer::PodVariant.freeze
-
-        it 'returns scopes by platform names if they qualify' do
-          variants = [
-            PodVariant.new([@root_spec], Platform.ios),
-            PodVariant.new([@root_spec], Platform.osx),
-          ]
-          @analyzer.send(:scope_suffixes_for_variants, variants).values.should == %w(iOS OSX)
-        end
-
-        it 'returns scopes by versioned platform names if they qualify' do
-          variants = [
-            PodVariant.new([@root_spec], Platform.ios),
-            PodVariant.new([@root_spec], Platform.new(:ios, '7.0')),
-          ]
-          @analyzer.send(:scope_suffixes_for_variants, variants).values.should == ['iOS', 'iOS7.0']
-        end
-
-        it 'returns scopes by subspec names if they qualify' do
-          shared_subspec = stub(:name => 'Spec/Shared', :root? => false)
-          variants = [
-            PodVariant.new([@root_spec, shared_subspec], Platform.ios),
-            PodVariant.new([@root_spec, shared_subspec, stub(:name => 'Spec/Foo', :root? => false)], Platform.ios),
-            PodVariant.new([@root_spec, shared_subspec, stub(:name => 'Spec/Bar', :root? => false)], Platform.ios),
-          ]
-          @analyzer.send(:scope_suffixes_for_variants, variants).values.should == [nil, 'Foo', 'Bar']
-        end
-
-        it 'returns scopes by subspec names if they qualify and handle partial root spec presence well' do
-          variants = [
-            PodVariant.new([stub(:name => 'Spec/Foo', :root? => false)], Platform.ios),
-            PodVariant.new([@root_spec, stub(:name => 'Spec/Bar', :root? => false)], Platform.ios),
-          ]
-          @analyzer.send(:scope_suffixes_for_variants, variants).values.should == ['Foo', 'Bar-root']
-        end
-
-        it 'returns scopes by platform names and subspec names if they qualify' do
-          variants = [
-            PodVariant.new([@root_spec], Platform.ios),
-            PodVariant.new([@root_spec, stub(:name => 'Spec/Foo', :root? => false)], Platform.ios),
-            PodVariant.new([@root_spec], Platform.osx),
-            PodVariant.new([@root_spec, stub(:name => 'Spec/Bar', :root? => false)], Platform.osx),
-          ]
-          @analyzer.send(:scope_suffixes_for_variants, variants).values.should == [
-            'iOS',
-            'iOS-Foo',
-            'OSX',
-            'OSX-Bar',
-          ]
-        end
-
-        it 'returns scopes by versioned platform names and subspec names if they qualify' do
-          specs = [
-            PodVariant.new([@root_spec], Platform.new(:ios, '7.0')),
-            PodVariant.new([@root_spec, stub(:name => 'Spec/Foo', :root? => false)], Platform.ios),
-            PodVariant.new([@root_spec], Platform.osx),
-            PodVariant.new([@root_spec, stub(:name => 'Spec/Bar', :root? => false)], Platform.osx),
-          ]
-          @analyzer.send(:scope_suffixes_for_variants, specs).values.should == [
-            'iOS7.0',
-            'iOS-Foo',
-            'OSX',
-            'OSX-Bar',
-          ]
-        end
-      end
-
-      #--------------------------------------#
-
       it 'generates the model to represent the target definitions' do
         target = @analyzer.analyze.targets.first
         target.pod_targets.map(&:name).sort.should == %w(
@@ -313,11 +238,11 @@ module Pod
           result = analyzer.analyze
 
           pod_targets = result.targets.flat_map(&:pod_targets).uniq.sort_by(&:name)
-          Hash[pod_targets.map { |t| [[t.label, t.requires_frameworks?], t.target_definitions.map(&:label)] }].should == {
-            ['BananaLib', false] => %w(Pods-SampleProject-TestRunner),
-            ['BananaLib', true]  => %w(Pods-SampleProject),
-            ['monkey', false]    => %w(Pods-SampleProject-TestRunner),
-            ['monkey', true]     => %w(Pods-SampleProject),
+          Hash[pod_targets.map { |t| [t.label, t.target_definitions.map(&:label)] }].should == {
+            'BananaLib-library'   => %w(Pods-SampleProject-TestRunner),
+            'BananaLib-framework' => %w(Pods-SampleProject),
+            'monkey-library'      => %w(Pods-SampleProject-TestRunner),
+            'monkey-framework'    => %w(Pods-SampleProject),
           }
         end