Launchpad itself

Merge ~cjwatson/launchpad:bugbear-getattr-setattr into launchpad:master

  1. Git
  2. lp:~cjwatson/launchpad
  3. bugbear-getattr-setattr
  4. Merge into master
Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: 39015f0c2ec9c6f2bf3fe940bca22d605c566814
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad:bugbear-getattr-setattr
Merge into: launchpad:master
Diff against target: 241 lines (+23/-34)
9 files modified
lib/lp/answers/tests/test_question_workflow.py (+10/-14)
lib/lp/blueprints/model/sprint.py (+1/-1)
lib/lp/code/model/branchtarget.py (+2/-8)
lib/lp/codehosting/vfs/branchfs.py (+1/-1)
lib/lp/codehosting/vfs/transport.py (+1/-1)
lib/lp/registry/tests/test_product.py (+4/-4)
lib/lp/services/webapp/error.py (+1/-1)
lib/lp/services/webapp/publisher.py (+1/-1)
lib/lp/testing/factory.py (+2/-3)
Reviewer Review Type Date Requested Status
Jürgen Gmach Approve
Review via email: mp+454020@code.launchpad.net

Commit message

Remove useless uses of getattr/setattr

Description of the change

`flake8-bugbear` points out:

  B009: Do not call getattr(x, 'attr'), instead use normal property access: x.attr. Missing a default to getattr will cause an AttributeError to be raised for non-existent properties. There is no additional safety in using getattr if you know the attribute name ahead of time.

  B010: Do not call setattr(x, 'attr', val), instead use normal property access: x.attr = val. There is no additional safety in using setattr if you know the attribute name ahead of time.

To post a comment you must log in.
Revision history for this message
Jürgen Gmach (jugmac00) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/answers/tests/test_question_workflow.py b/lib/lp/answers/tests/test_question_workflow.py
2index c0e61ce..2569042 100644
3--- a/lib/lp/answers/tests/test_question_workflow.py
4+++ b/lib/lp/answers/tests/test_question_workflow.py
5@@ -426,7 +426,7 @@ class RequestInfoTestCase(BaseAnswerTrackerWorkflowTestCase):
6 self.assertRaises(Unauthorized, getattr, self.question, "requestInfo")
7
8 login_person(self.answerer)
9- getattr(self.question, "requestInfo")
10+ self.question.requestInfo
11
12
13 class GiveInfoTestCase(BaseAnswerTrackerWorkflowTestCase):
14@@ -473,7 +473,7 @@ class GiveInfoTestCase(BaseAnswerTrackerWorkflowTestCase):
15 self.assertRaises(Unauthorized, getattr, self.question, "giveInfo")
16
17 login_person(self.owner)
18- getattr(self.question, "giveInfo")
19+ self.question.giveInfo
20
21
22 class GiveAnswerTestCase(BaseAnswerTrackerWorkflowTestCase):
23@@ -588,7 +588,7 @@ class GiveAnswerTestCase(BaseAnswerTrackerWorkflowTestCase):
24 self.assertRaises(Unauthorized, getattr, self.question, "giveAnswer")
25
26 login_person(self.answerer)
27- getattr(self.question, "giveAnswer")
28+ self.question.giveAnswer
29
30
31 class LinkFAQTestCase(BaseAnswerTrackerWorkflowTestCase):
32@@ -680,7 +680,7 @@ class LinkFAQTestCase(BaseAnswerTrackerWorkflowTestCase):
33 self.assertRaises(Unauthorized, getattr, self.question, "linkFAQ")
34
35 login_person(self.answerer)
36- getattr(self.question, "linkFAQ")
37+ self.question.linkFAQ
38
39
40 class ConfirmAnswerTestCase(BaseAnswerTrackerWorkflowTestCase):
41@@ -856,7 +856,7 @@ class ConfirmAnswerTestCase(BaseAnswerTrackerWorkflowTestCase):
42 )
43
44 login_person(self.owner)
45- getattr(self.question, "confirmAnswer")
46+ self.question.confirmAnswer
47
48
49 class ReopenTestCase(BaseAnswerTrackerWorkflowTestCase):
50@@ -971,7 +971,7 @@ class ReopenTestCase(BaseAnswerTrackerWorkflowTestCase):
51 self.assertRaises(Unauthorized, getattr, self.question, "reopen")
52
53 login_person(self.owner)
54- getattr(self.question, "reopen")
55+ self.question.reopen
56
57
58 class ExpireQuestionTestCase(BaseAnswerTrackerWorkflowTestCase):
59@@ -1014,7 +1014,7 @@ class ExpireQuestionTestCase(BaseAnswerTrackerWorkflowTestCase):
60 )
61
62 login_person(self.answerer)
63- getattr(self.question, "expireQuestion")
64+ self.question.expireQuestion
65
66
67 class RejectTestCase(BaseAnswerTrackerWorkflowTestCase):
68@@ -1103,13 +1103,10 @@ class RejectTestCase(BaseAnswerTrackerWorkflowTestCase):
69 # clear authorization cache for check_permission
70 clear_cache()
71 self.assertTrue(
72- getattr(self.question, "reject"),
73- "Answer contact cannot reject question.",
74+ self.question.reject, "Answer contact cannot reject question."
75 )
76 login_person(self.admin)
77- self.assertTrue(
78- getattr(self.question, "reject"), "Admin cannot reject question."
79- )
80+ self.assertTrue(self.question.reject, "Admin cannot reject question.")
81
82 def testRejectPermission_indirect_answer_contact(self):
83 # Indirect answer contacts (for a distribution) can reject
84@@ -1121,6 +1118,5 @@ class RejectTestCase(BaseAnswerTrackerWorkflowTestCase):
85 self.answerer.addLanguage(getUtility(ILanguageSet)["en"])
86 self.ubuntu.addAnswerContact(self.answerer, self.answerer)
87 self.assertTrue(
88- getattr(self.question, "reject"),
89- "Answer contact cannot reject question.",
90+ self.question.reject, "Answer contact cannot reject question."
91 )
92diff --git a/lib/lp/blueprints/model/sprint.py b/lib/lp/blueprints/model/sprint.py
93index 15f20ea..7b5fc16 100644
94--- a/lib/lp/blueprints/model/sprint.py
95+++ b/lib/lp/blueprints/model/sprint.py
96@@ -425,7 +425,7 @@ class HasSprintsMixin:
97
98 Subclasses must overwrite this method if it doesn't suit them.
99 """
100- table = getattr(self, "__storm_table__")
101+ table = self.__storm_table__
102 return [
103 getattr(Specification, table.lower()) == self,
104 Specification.id == SprintSpecification.specification_id,
105diff --git a/lib/lp/code/model/branchtarget.py b/lib/lp/code/model/branchtarget.py
106index 998c391..ceb60fa 100644
107--- a/lib/lp/code/model/branchtarget.py
108+++ b/lib/lp/code/model/branchtarget.py
109@@ -184,10 +184,7 @@ class PackageBranchTarget(_BaseBranchTarget):
110
111 result = sorted_version_numbers(
112 result,
113- key=lambda branch_info: (
114- getattr(branch_info[1], "name"),
115- branch_info[0].id,
116- ),
117+ key=lambda branch_info: (branch_info[1].name, branch_info[0].id),
118 )
119
120 if limit_results is not None:
121@@ -438,10 +435,7 @@ class ProductBranchTarget(_BaseBranchTarget):
122
123 result = sorted_version_numbers(
124 result,
125- key=lambda branch_info: (
126- getattr(branch_info[1], "name"),
127- branch_info[0].id,
128- ),
129+ key=lambda branch_info: (branch_info[1].name, branch_info[0].id),
130 )
131
132 if limit_results is not None:
133diff --git a/lib/lp/codehosting/vfs/branchfs.py b/lib/lp/codehosting/vfs/branchfs.py
134index 74461c9..3010e69 100644
135--- a/lib/lp/codehosting/vfs/branchfs.py
136+++ b/lib/lp/codehosting/vfs/branchfs.py
137@@ -502,7 +502,7 @@ class AsyncLaunchpadTransport(AsyncVirtualTransport):
138 @no_traceback_failures
139 def real_mkdir(result):
140 transport, path = result
141- return getattr(transport, "mkdir")(path, mode)
142+ return transport.mkdir(path, mode)
143
144 deferred.addCallback(real_mkdir)
145 deferred.addErrback(maybe_make_branch_in_db)
146diff --git a/lib/lp/codehosting/vfs/transport.py b/lib/lp/codehosting/vfs/transport.py
147index bcdb9d3..b896c65 100644
148--- a/lib/lp/codehosting/vfs/transport.py
149+++ b/lib/lp/codehosting/vfs/transport.py
150@@ -247,7 +247,7 @@ class AsyncVirtualTransport(Transport):
151 "cannot move between underlying transports"
152 )
153 )
154- return getattr(from_transport, "rename")(from_path, to_path)
155+ return from_transport.rename(from_path, to_path)
156
157 deferred.addCallback(check_transports_and_rename)
158 return deferred
159diff --git a/lib/lp/registry/tests/test_product.py b/lib/lp/registry/tests/test_product.py
160index 4b22c33..1ef5135 100644
161--- a/lib/lp/registry/tests/test_product.py
162+++ b/lib/lp/registry/tests/test_product.py
163@@ -1443,9 +1443,9 @@ class TestProduct(TestCaseWithFactory):
164 )
165 ordinary_user = self.factory.makePerson()
166 with person_logged_in(ordinary_user):
167- setattr(product, "date_next_suggest_packaging", "foo")
168+ product.date_next_suggest_packaging = "foo"
169 with person_logged_in(product.owner):
170- setattr(product, "date_next_suggest_packaging", "foo")
171+ product.date_next_suggest_packaging = "foo"
172
173 def test_set_launchpad_AnyAllowedPerson_proprietary_product(self):
174 # Only people with grants for a private product can set
175@@ -1473,7 +1473,7 @@ class TestProduct(TestCaseWithFactory):
176 "foo",
177 )
178 with person_logged_in(owner):
179- setattr(product, "date_next_suggest_packaging", "foo")
180+ product.date_next_suggest_packaging = "foo"
181 # A user with a policy grant for the product can access attributes
182 # of a private product.
183 with person_logged_in(owner):
184@@ -1484,7 +1484,7 @@ class TestProduct(TestCaseWithFactory):
185 {InformationType.PROPRIETARY: SharingPermission.ALL},
186 )
187 with person_logged_in(ordinary_user):
188- setattr(product, "date_next_suggest_packaging", "foo")
189+ product.date_next_suggest_packaging = "foo"
190
191 def test_userCanView_caches_known_users(self):
192 # userCanView() maintains a cache of users known to have the
193diff --git a/lib/lp/services/webapp/error.py b/lib/lp/services/webapp/error.py
194index 4c451f9..b0f7f3f 100644
195--- a/lib/lp/services/webapp/error.py
196+++ b/lib/lp/services/webapp/error.py
197@@ -67,7 +67,7 @@ class SystemErrorView(LaunchpadView):
198 self.request.response.removeAllNotifications()
199 if self.response_code is not None:
200 self.request.response.setStatus(self.response_code)
201- if getattr(self.request, "oopsid") is not None:
202+ if self.request.oopsid is not None:
203 self.request.response.addHeader(
204 "X-Lazr-OopsId", self.request.oopsid
205 )
206diff --git a/lib/lp/services/webapp/publisher.py b/lib/lp/services/webapp/publisher.py
207index 4702b67..bf64b0d 100644
208--- a/lib/lp/services/webapp/publisher.py
209+++ b/lib/lp/services/webapp/publisher.py
210@@ -185,7 +185,7 @@ class redirection:
211 redirections = getattr(fn, "__redirections__", None)
212 if redirections is None:
213 redirections = {}
214- setattr(fn, "__redirections__", redirections)
215+ fn.__redirections__ = redirections
216 redirections[self.name] = self.status
217 return fn
218
219diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
220index 7605ed7..71fac63 100644
221--- a/lib/lp/testing/factory.py
222+++ b/lib/lp/testing/factory.py
223@@ -1552,8 +1552,7 @@ class LaunchpadObjectFactory(ObjectFactory):
224
225 # Sort them
226 related_series_branch_info = sorted_version_numbers(
227- series_branch_info,
228- key=lambda branch_info: (getattr(branch_info[1], "name")),
229+ series_branch_info, key=lambda branch_info: branch_info[1].name
230 )
231
232 # Add a development branch at the start of the list.
233@@ -1641,7 +1640,7 @@ class LaunchpadObjectFactory(ObjectFactory):
234
235 related_package_branch_info = sorted_version_numbers(
236 related_package_branch_info,
237- key=lambda branch_info: (getattr(branch_info[1], "name")),
238+ key=lambda branch_info: branch_info[1].name,
239 )
240
241 return (

Subscribers

People subscribed via source and target branches

to status/vote changes: