Repository: scrapy/scrapy

Issue:
Idea: warn users when trying to use TextResponse functionality with plain Response
Currently, if we try to use TextResponse functionality like response.text or css()/xpath() methods with a plain Response (e.g. in case of binary content), we get an AttributeError:

```
>>> response.css
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-1-7d6e256164d4> in <module>()
----> 1 response.css

AttributeError: 'Response' object has no attribute 'css'
>>> response.xpath
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-2-4f61f6e9fc6e> in <module>()
----> 1 response.xpath

AttributeError: 'Response' object has no attribute 'xpath'
>>> response.text
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-3-be6a4a00df5e> in <module>()
----> 1 response.text

AttributeError: 'Response' object has no attribute 'text'
```

Would it make sense to add a few methods/properties to explain what's going on for new users?

I was thinking instead of AttributeError, a better behavior could be a ValueError with a message giving a bit more context.

So, in plain `Response`, we could have:

```
def css(self, *args, **kw):
    raise ValueError('Response content is not text')

def xpath(self, *args, **kw):
    raise ValueError('Response content is not text')

@property
def text(self, *args, **kw):
    raise ValueError('Response content is not text')
```

This would be nice, because we'd had to explain fewer things when teaching people about responses and also about using `.css` and `.xpath` methods.

What do you think?


Discussion:

Patch:
diff --git a/scrapy/http/response/__init__.py b/scrapy/http/response/__init__.py
index 983154001..58ad414f1 100644
--- a/scrapy/http/response/__init__.py
+++ b/scrapy/http/response/__init__.py
@@ -9,6 +9,8 @@ from six.moves.urllib.parse import urljoin
 from scrapy.http.headers import Headers
 from scrapy.utils.trackref import object_ref
 from scrapy.http.common import obsolete_setter
+from scrapy.exceptions import NotSupported
+
 
 class Response(object_ref):
 
@@ -80,3 +82,22 @@ class Response(object_ref):
         """Join this Response's url with a possible relative url to form an
         absolute interpretation of the latter."""
         return urljoin(self.url, url)
+
+    @property
+    def text(self):
+        """For subclasses of TextResponse, this will return the body
+        as text (unicode object in Python 2 and str in Python 3)
+        """
+        raise AttributeError("Response content isn't text")
+
+    def css(self, *a, **kw):
+        """Shortcut method implemented only by responses whose content
+        is text (subclasses of TextResponse).
+        """
+        raise NotSupported("Response content isn't text")
+
+    def xpath(self, *a, **kw):
+        """Shortcut method implemented only by responses whose content
+        is text (subclasses of TextResponse).
+        """
+        raise NotSupported("Response content isn't text")


Test patch:
diff --git a/tests/test_http_response.py b/tests/test_http_response.py
index c7f36687a..7624aa4c4 100644
--- a/tests/test_http_response.py
+++ b/tests/test_http_response.py
@@ -7,6 +7,7 @@ from scrapy.http import (Request, Response, TextResponse, HtmlResponse,
                          XmlResponse, Headers)
 from scrapy.selector import Selector
 from scrapy.utils.python import to_native_str
+from scrapy.exceptions import NotSupported
 
 
 class BaseResponseTest(unittest.TestCase):
@@ -127,6 +128,18 @@ class BaseResponseTest(unittest.TestCase):
         absolute = 'http://www.example.com/test'
         self.assertEqual(joined, absolute)
 
+    def test_shortcut_attributes(self):
+        r = self.response_class("http://example.com", body=b'hello')
+        if self.response_class == Response:
+            msg = "Response content isn't text"
+            self.assertRaisesRegexp(AttributeError, msg, getattr, r, 'text')
+            self.assertRaisesRegexp(NotSupported, msg, r.css, 'body')
+            self.assertRaisesRegexp(NotSupported, msg, r.xpath, '//body')
+        else:
+            r.text
+            r.css('body')
+            r.xpath('//body')
+
 
 class TextResponseTest(BaseResponseTest):