Diff
Modified: trunk/Tools/ChangeLog (200858 => 200859)
--- trunk/Tools/ChangeLog 2016-05-13 16:06:03 UTC (rev 200858)
+++ trunk/Tools/ChangeLog 2016-05-13 16:16:51 UTC (rev 200859)
@@ -1,3 +1,16 @@
+2016-05-13 Brady Eidson <beid...@apple.com>
+
+ Protector Ref/RefPtrs should have a specified naming style.
+ https://bugs.webkit.org/show_bug.cgi?id=157591
+
+ Reviewed by Darin Adler.
+
+ * Scripts/webkitpy/style/checkers/cpp.py:
+ (check_identifier_name_in_declaration):
+ (CppChecker):
+ * Scripts/webkitpy/style/checkers/cpp_unittest.py:
+ (WebKitStyleTest.test_names):
+
2016-05-12 Csaba Osztrogonác <o...@webkit.org>
Remove ENABLE(ES6_ARROWFUNCTION_SYNTAX) guards
Modified: trunk/Tools/Scripts/webkitpy/style/checkers/cpp.py (200858 => 200859)
--- trunk/Tools/Scripts/webkitpy/style/checkers/cpp.py 2016-05-13 16:06:03 UTC (rev 200858)
+++ trunk/Tools/Scripts/webkitpy/style/checkers/cpp.py 2016-05-13 16:16:51 UTC (rev 200859)
@@ -3291,6 +3291,21 @@
if match(r'\s*(return|delete)\b', line):
return
+ # Make sure Ref/RefPtrs used as protectors are named correctly, and do this before we start stripping things off the input.
+ ref_regexp = r'^\s*Ref(Ptr)?<([\w_]|::)+> (?P<protector_name>[\w_]+)\((\*|&)*(m_)?(?P<protected_name>[\w_]+)\);'
+ ref_check = match(ref_regexp, line)
+ if ref_check:
+ protector_name = ref_check.group('protector_name')
+ protected_name = ref_check.group('protected_name')
+ cap_protected_name = protected_name[0].upper() + protected_name[1:]
+ expected_protector_name = 'protected' + cap_protected_name
+ if protected_name == 'this' and protector_name != 'protectedThis':
+ error(line_number, 'readability/naming/protected', 4, "\'" + protector_name + "\' is incorrectly named. It should be named \'protectedThis\'.")
+ elif protector_name == expected_protector_name or protector_name == 'protector':
+ return
+ else:
+ error(line_number, 'readability/naming/protected', 4, "\'" + protector_name + "\' is incorrectly named. It should be named \'protector\' or \'" + expected_protector_name + "\'.")
+
# Basically, a declaration is a type name followed by whitespaces
# followed by an identifier. The type name can be complicated
# due to type adjectives and templates. We remove them first to
@@ -3846,6 +3861,7 @@
'readability/multiline_string',
'readability/parameter_name',
'readability/naming',
+ 'readability/naming/protected',
'readability/naming/underscores',
'readability/null',
'readability/streams',
Modified: trunk/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py (200858 => 200859)
--- trunk/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py 2016-05-13 16:06:03 UTC (rev 200858)
+++ trunk/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py 2016-05-13 16:16:51 UTC (rev 200859)
@@ -5179,6 +5179,52 @@
self.assert_lint('unsigned _fillRule : 1;',
'_fillRule' + name_underscore_error_message)
+ # Valid protector RefPtr/Ref variable names.
+ self.assert_lint('RefPtr<Node> protectedThis(this);', '')
+ self.assert_lint('Ref<Node> protectedThis(*this);', '')
+ self.assert_lint('RefPtr<Node> protectedNode(node);', '')
+ self.assert_lint('RefPtr<Node> protectedNode(&node);', '')
+ self.assert_lint('RefPtr<Node> protector(node);', '')
+ self.assert_lint('RefPtr<Node> protector(&node);', '')
+ self.assert_lint('Ref<Node> protectedNode(node);', '')
+ self.assert_lint('Ref<Node> protectedNode(*node);', '')
+ self.assert_lint('Ref<Node> protector(node);', '')
+ self.assert_lint('Ref<Node> protector(*node);', '')
+ self.assert_lint('RefPtr<Node> protectedOtherNode(otherNode);', '')
+ self.assert_lint('RefPtr<Node> protectedOtherNode(&otherNode);', '')
+ self.assert_lint('Ref<Node> protectedOtherNode(otherNode);', '')
+ self.assert_lint('Ref<Node> protectedOtherNode(*otherNode);', '')
+ self.assert_lint('RefPtr<Widget> protectedWidget(m_widget);', '')
+ self.assert_lint('RefPtr<Widget> protectedWidget(&m_widget);', '')
+ self.assert_lint('Ref<Widget> protectedWidget(m_widget);', '')
+ self.assert_lint('Ref<Widget> protectedWidget(*m_widget);', '')
+ self.assert_lint('RefPtr<Widget> protector(m_widget);', '')
+ self.assert_lint('RefPtr<Widget> protector(&m_widget);', '')
+ self.assert_lint('Ref<Widget> protector(m_widget);', '')
+ self.assert_lint('Ref<Widget> protector(*m_widget);', '')
+ self.assert_lint('RefPtr<SomeNamespace::Node> protectedThis(this);', '')
+ self.assert_lint('RefPtr<SomeClass::InternalClass::Node> protectedThis(this);', '')
+
+ # Invalid protector RefPtr/Ref variable names.
+ self.assert_lint('RefPtr<Node> protector(this);', "'protector' is incorrectly named. It should be named 'protectedThis'. [readability/naming/protected] [4]")
+ self.assert_lint('Ref<Node> protector(*this);', "'protector' is incorrectly named. It should be named 'protectedThis'. [readability/naming/protected] [4]")
+ self.assert_lint('RefPtr<Node> self(this);', "'self' is incorrectly named. It should be named 'protectedThis'. [readability/naming/protected] [4]")
+ self.assert_lint('Ref<Node> self(*this);', "'self' is incorrectly named. It should be named 'protectedThis'. [readability/naming/protected] [4]")
+ self.assert_lint('RefPtr<Node> protectedThis(node);', "'protectedThis' is incorrectly named. It should be named 'protector' or 'protectedNode'. [readability/naming/protected] [4]")
+ self.assert_lint('RefPtr<Node> protectedThis(&node);', "'protectedThis' is incorrectly named. It should be named 'protector' or 'protectedNode'. [readability/naming/protected] [4]")
+ self.assert_lint('Ref<Node> protectedThis(node);', "'protectedThis' is incorrectly named. It should be named 'protector' or 'protectedNode'. [readability/naming/protected] [4]")
+ self.assert_lint('Ref<Node> protectedThis(*node);', "'protectedThis' is incorrectly named. It should be named 'protector' or 'protectedNode'. [readability/naming/protected] [4]")
+ self.assert_lint('RefPtr<Node> protectedNode(otherNode);', "'protectedNode' is incorrectly named. It should be named 'protector' or 'protectedOtherNode'. [readability/naming/protected] [4]")
+ self.assert_lint('RefPtr<Node> protectedNode(&otherNode);', "'protectedNode' is incorrectly named. It should be named 'protector' or 'protectedOtherNode'. [readability/naming/protected] [4]")
+ self.assert_lint('Ref<Node> protectedNode(otherNode);', "'protectedNode' is incorrectly named. It should be named 'protector' or 'protectedOtherNode'. [readability/naming/protected] [4]")
+ self.assert_lint('Ref<Node> protectedNode(*otherNode);', "'protectedNode' is incorrectly named. It should be named 'protector' or 'protectedOtherNode'. [readability/naming/protected] [4]")
+ self.assert_lint('RefPtr<Node> nodeRef(node);', "'nodeRef' is incorrectly named. It should be named 'protector' or 'protectedNode'. [readability/naming/protected] [4]")
+ self.assert_lint('Ref<Node> nodeRef(*node);', "'nodeRef' is incorrectly named. It should be named 'protector' or 'protectedNode'. [readability/naming/protected] [4]")
+
+ # Lines that look like a protector variable declaration but aren't.
+ self.assert_lint('static RefPtr<Widget> doSomethingWith(widget);', '')
+ self.assert_lint('RefPtr<Widget> create();', '')
+
def test_parameter_names(self):
# Leave meaningless variable names out of function declarations.
meaningless_variable_name_error_message = 'The parameter name "%s" adds no information, so it should be removed. [readability/parameter_name] [5]'
Modified: trunk/Websites/webkit.org/ChangeLog (200858 => 200859)
--- trunk/Websites/webkit.org/ChangeLog 2016-05-13 16:06:03 UTC (rev 200858)
+++ trunk/Websites/webkit.org/ChangeLog 2016-05-13 16:16:51 UTC (rev 200859)
@@ -1,3 +1,12 @@
+2016-05-13 Brady Eidson <beid...@apple.com>
+
+ Protector Ref/RefPtrs should have a specified naming style.
+ https://bugs.webkit.org/show_bug.cgi?id=157591
+
+ Reviewed by Darin Adler.
+
+ * code-style.md:
+
2016-04-26 Timothy Hatcher <timo...@apple.com>
Remove hard-wraps from the feature policy markdown so it renders full width on the site.
Modified: trunk/Websites/webkit.org/code-style.md (200858 => 200859)
--- trunk/Websites/webkit.org/code-style.md 2016-05-13 16:06:03 UTC (rev 200858)
+++ trunk/Websites/webkit.org/code-style.md 2016-05-13 16:16:51 UTC (rev 200859)
@@ -713,6 +713,42 @@
#define _HTML_DOCUMENT_H_
```
+[](#names-protectors-this) Ref and RefPtr objects meant to protect `this` from deletion should be named "protectedThis".
+
+###### Right:
+
+```cpp
+RefPtr<Node> protectedThis(this);
+Ref<Element> protectedThis(*this);
+```
+
+###### Wrong:
+
+```cpp
+RefPtr<Node> protector(this);
+RefPtr<Widget> self(this);
+Ref<Element> elementRef(*this);
+```
+
+[](#names-protectors) Ref and RefPtr objects meant to protect variables other than `this` from deletion should be named either "protector", or "protected" combined with the capitalized form of the variable name.
+
+###### Right:
+
+```cpp
+RefPtr<Element> protector(&element);
+RefPtr<Node> protectedNode(node);
+RefPtr<Widget> protectedMainWidget(m_mainWidget);
+```
+
+###### Wrong:
+
+```cpp
+RefPtr<Node> nodeRef(&rootNode);
+Ref<Element> protect(*element);
+RefPtr<Widget> protected(widget);
+RefPtr<Node> protectorNode(node);
+```
+
### Other Punctuation
[](#punctuation-member-init) Constructors for C++ classes should initialize all of their members using C++ initializer syntax. Each member (and superclass) should be indented on a separate line, with the colon or comma preceding the member on that line.
