[Sass] Be smarter about what counts as a CSS import.

Closes gh-177

doc-src/SASS_CHANGELOG.md

@@ -5,6 +5,18 @@
 
 ## 3.0.13 (Unreleased)
 
+## CSS `@import` Directives
+
+Sass is now more intelligent about when to compile `@import` directives to plain CSS.
+Any of the following conditions will cause a literal CSS `@import`:
+
+* Importing a path with a `.css` extension (e.g. `@import "foo.css"`).
+* Importing a path with a media type (e.g. `@import "foo" screen;`).
+* Importing an HTTP path (e.g. `@import "http://foo.com/style.css"`).
+* Importing any URL (e.g. `@import url(foo)`).
+
+The former two conditions always worked, but the latter two are new.
+
 ## Rails 3 Support
 
 Support for Rails 3 versions prior to beta 4 has been removed.

lib/sass/engine.rb

@@ -509,16 +509,7 @@ WARNING
       # If value begins with url( or ",
       # it's a CSS @import rule and we don't want to touch it.
       if directive == "import"
-        raise SyntaxError.new("Illegal nesting: Nothing may be nested beneath import directives.",
-          :line => @line + 1) unless line.children.empty?
-        if (match = value.match(Sass::SCSS::RX::STRING) || value.match(Sass::SCSS::RX::URI)) &&
-            !match.post_match.strip.empty? && match.post_match.strip[0] != ?,
-          return Tree::DirectiveNode.new("@import #{value}")
-        end
-        value.split(/,\s*/).map do |f|
-          f = $1 || $2 || $3 if f =~ Sass::SCSS::RX::STRING || f =~ Sass::SCSS::RX::URI
-          Tree::ImportNode.new(f)
-        end
+        parse_import(line, value)
       elsif directive == "mixin"
         parse_mixin_definition(line)
       elsif directive == "include"
@@ -597,6 +588,32 @@ WARNING
       nil
     end
 
+    def parse_import(line, value)
+      raise SyntaxError.new("Illegal nesting: Nothing may be nested beneath import directives.",
+        :line => @line + 1) unless line.children.empty?
+
+      if (match = value.match(Sass::SCSS::RX::STRING) || value.match(Sass::SCSS::RX::URI)) &&
+          match.offset(0).first == 0 && !match.post_match.strip.empty? &&
+          match.post_match.strip[0] != ?,
+        # @import "filename" media-type
+        return Tree::DirectiveNode.new("@import #{value}")
+      end
+
+      value.split(/,\s*/).map do |f|
+        if f =~ Sass::SCSS::RX::URI
+          # All url()s are literal CSS @imports
+          next Tree::DirectiveNode.new("@import #{f}")
+        elsif f =~ Sass::SCSS::RX::STRING
+          f = $1 || $2
+        end
+
+        # http:// URLs are always literal CSS imports
+        next Tree::DirectiveNode.new("@import url(#{f})") if f =~ /^http:\/\//
+
+        Tree::ImportNode.new(f)
+      end
+    end
+
     MIXIN_DEF_RE = /^(?:=|@mixin)\s*(#{Sass::SCSS::RX::IDENT})(.*)$/
     def parse_mixin_definition(line)
       name, arg_string = line.text.scan(MIXIN_DEF_RE).first

lib/sass/scss/parser.rb

@@ -200,13 +200,13 @@ module Sass
 
       def import_directive
         @expected = "string or url()"
-        arg = tok(STRING) || tok!(URI)
+        arg = tok(STRING) || (uri = tok!(URI))
         path = @scanner[1] || @scanner[2] || @scanner[3]
         ss
 
         media = str {media_query_list}.strip
 
-        if !media.strip.empty? || use_css_import?
+        if uri || path =~ /^http:\/\// || !media.strip.empty? || use_css_import?
           return node(Sass::Tree::DirectiveNode.new("@import #{arg} #{media}".strip))
         end
 

test/sass/conversion_test.rb

@@ -671,11 +671,15 @@ SCSS
     assert_renders <<SASS, <<SCSS
 @import foo
 
+@import url(bar.css)
+
 foo
   bar: baz
 SASS
 @import "foo";
 
+@import url(bar.css);
+
 foo {
   bar: baz; }
 SCSS
@@ -683,11 +687,15 @@ SCSS
     assert_renders <<SASS, <<SCSS
 @import foo.css
 
+@import url(bar.css)
+
 foo
   bar: baz
 SASS
 @import "foo.css";
 
+@import url(bar.css);
+
 foo {
   bar: baz; }
 SCSS
@@ -695,7 +703,6 @@ SCSS
 
   def test_import_as_directive_in_sass
     assert_equal "@import foo.css\n", to_sass('@import "foo.css"')
-    assert_equal "@import foo.css\n", to_sass('@import url(foo.css)')
   end
 
   def test_import_as_directive_in_scss

test/sass/engine_test.rb

@@ -487,8 +487,21 @@ CSS
   end
 
   def test_css_import
-    assert_equal("@import url(./fonts.css) screen;\n", render("@import url(./fonts.css) screen"))
-    assert_equal("@import \"./fonts.css\" screen;\n", render("@import \"./fonts.css\" screen"))
+    assert_equal("@import url(./fonts.css);\n", render("@import \"./fonts.css\""))
+  end
+
+  def test_media_import
+    assert_equal("@import \"./fonts.sass\" all;\n",
+      render("@import \"./fonts.sass\" all"))
+  end
+
+  def test_http_import
+    assert_equal("@import url(http://fonts.googleapis.com/css?family=Droid+Sans);\n",
+      render("@import \"http://fonts.googleapis.com/css?family=Droid+Sans\""))
+  end
+
+  def test_url_import
+    assert_equal("@import url(fonts.sass);\n", render("@import url(fonts.sass)"))
   end
 
   def test_sass_import

test/sass/scss/scss_test.rb

@@ -212,11 +212,24 @@ SCSS
   def test_css_import_directive
     assert_equal "@import url(foo.css);\n", render('@import "foo.css";')
     assert_equal "@import url(foo.css);\n", render("@import 'foo.css';")
-    assert_equal "@import url(foo.css);\n", render('@import url("foo.css");')
-    assert_equal "@import url(foo.css);\n", render("@import url('foo.css');")
+    assert_equal "@import url(\"foo.css\");\n", render('@import url("foo.css");')
+    assert_equal "@import url('foo.css');\n", render("@import url('foo.css');")
     assert_equal "@import url(foo.css);\n", render('@import url(foo.css);')
   end
 
+  def test_media_import
+    assert_equal("@import \"./fonts.sass\" all;\n", render("@import \"./fonts.sass\" all;"))
+  end
+
+  def test_http_import
+    assert_equal("@import \"http://fonts.googleapis.com/css?family=Droid+Sans\";\n",
+      render("@import \"http://fonts.googleapis.com/css?family=Droid+Sans\";"))
+  end
+
+  def test_url_import
+    assert_equal("@import url(fonts.sass);\n", render("@import url(fonts.sass);"))
+  end
+
   def test_block_comment_in_script
     assert_equal <<CSS, render(<<SCSS)
 foo {

test/sass/templates/import.sass

@@ -4,7 +4,7 @@ $preconst: hello
   pre-mixin: here
 
 @import importee.sass, scss_importee, "basic.sass", basic.css, ../results/complex.css
-@import url(partial.sass)
+@import partial.sass
 
 nonimported
   :myconst $preconst