Skip to content

Commit

Permalink
Disallow $parent and $broadcast angularJs property (oppia#13291)
Browse files Browse the repository at this point in the history
  • Loading branch information
AdityaDubey0 authored Jul 5, 2021
1 parent 35302c4 commit 90d0152
Show file tree
Hide file tree
Showing 8 changed files with 137 additions and 101 deletions.
1 change: 1 addition & 0 deletions .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@
"oppia/constant-declaration": "error",
"oppia/comment-style": "error",
"oppia/disallow-flags": "error",
"oppia/disallow-angularjs-properties": "error",
"oppia/no-multiple-component": "error",
"oppia/no-multiline-disable": "error",
"oppia/directive-scope": "error",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ describe('Audio translation bar directive', function() {
}));

afterEach(function() {
// eslint-disable-next-line oppia/disallow-angularjs-properties
$rootScope.$broadcast('$destroy');
});

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// Copyright 2021 The Oppia Authors. All Rights Reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS-IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

/**
* @fileoverview Lint check to disallow $parent and $broadcast
* properties of angularJs.
*/

'use strict';

module.exports = {
meta: {
type: 'problem',
docs: {
description: (
'Lint check to disallow $parent and $broadcast properties of angularJs'
),
category: 'Best Practices',
recommended: true,
},
fixable: null,
schema: [],
messages: {
disallowBrodcast: (
'Please do not use $broadcast/$on for propagating' +
' events. Use @Input/@Output instead'),
disallowParent: (
'Please do not access parent properties using $parent.' +
' Use the scope object for this purpose.')
},
},

create: function(context) {
var checkAndReportAngularJsProperties = function(node) {
if (node.property.name === '$parent') {
context.report({
node: node,
messageId: 'disallowParent'
});
}
if (node.property.name === '$broadcast') {
context.report({
node: node,
messageId: 'disallowBrodcast'
});
}
};

return {
MemberExpression: function(node) {
checkAndReportAngularJsProperties(node);
}
};
}
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
// Copyright 2021 The Oppia Authors. All Rights Reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS-IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

/**
* @fileoverview Tests for the disallow-angularjs-properties.js file.
*/

'use strict';

var rule = require('./disallow-angularjs-properties');
var RuleTester = require('eslint').RuleTester;

var ruleTester = new RuleTester();
ruleTester.run('disallow-angularjs-properties', rule, {
valid: [
`
var ParentCtrl = function($scope) {
$scope.cities = ['NY', 'Amsterdam', 'Barcelona'];
};
`
],

invalid: [
{
code:
`
var ChildCtrl = function($scope) {
$scope.parentcities = $scope.$parent.cities;
};
`,
errors: [{
message: (
'Please do not access parent properties using $parent.' +
' Use the scope object for this purpose.')
}]
},
{
code:
`
angular.module('oppia').directive('sampleDirective', [
function() {
return {
controller: ['$rootScope',
function($rootScope) {
$rootScope.$broadcast('sampleEvent');
}
]
};
}]);
`,
errors: [{
message: (
'Please do not use $broadcast/$on for propagating events.' +
' Use @Input/@Output instead')
}]
}
]
});
21 changes: 0 additions & 21 deletions scripts/linters/general_purpose_linter.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,14 +139,6 @@
'extensions/value_generators/',
'extensions/visualizations/')
},
{
'regexp': re.compile(r'\$parent'),
'message': 'Please do not access parent properties ' +
'using $parent. Use the scope object' +
'for this purpose.',
'excluded_files': (),
'excluded_dirs': ()
},
{
'regexp': re.compile(r'require\(.*\.\..*\);'),
'message': 'Please, don\'t use relative imports in require().',
Expand All @@ -168,19 +160,6 @@
'rte-output-display.component.spec.ts'),
'excluded_dirs': ('core/tests/',)
},
{
'regexp': re.compile(r'\$broadcast'),
'message': (
'Please do not use $broadcast/$on for propagating events. '
'Use @Input/@Output instead.'),
'excluded_files': (
'core/templates/pages/exploration-editor-page/translation-tab/'
'audio-translation-bar/audio-translation-bar.directive.spec.ts',
'core/templates/pages/library-page/search-bar/'
'search-bar.component.spec.ts',
'core/templates/pages/splash-page/splash-page.component.spec.ts'),
'excluded_dirs': ()
},
{
'regexp': re.compile(r'import (\{.*\}|\_) from \'lodash\''),
'message': (
Expand Down
24 changes: 0 additions & 24 deletions scripts/linters/general_purpose_linter_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,10 @@
LINTER_TESTS_DIR, 'invalid_innerhtml.ts')
INVALID_RELATIVE_IMPORT_FILEPATH = os.path.join(
LINTER_TESTS_DIR, 'invalid_relative_import.js')
INVALID_PARENT_FILEPATH = os.path.join(LINTER_TESTS_DIR, 'invalid_parent.ts')
INVALID_TEMPLATE_URL_FILEPATH = os.path.join(
LINTER_TESTS_DIR, 'invalid_templateurl.ts')
INVALID_FILEOVERVIEW_FILEPATH = os.path.join(
LINTER_TESTS_DIR, 'invalid_fileoverview.ts')
INVALID_BROADCAST_USE_FILEPATH = os.path.join(
LINTER_TESTS_DIR, 'invalid_broadcast_use.ts')
INVALID_LODASH_GENERAL_IMPORT_FILEPATH = os.path.join(
LINTER_TESTS_DIR, 'invalid_lodash_general_import.ts')
INVALID_LODASH_SPECIFIC_IMPORT_FILEPATH = os.path.join(
Expand Down Expand Up @@ -159,17 +156,6 @@ def test_invalid_use_of_template_url(self):
self.assertEqual('Bad pattern', lint_task_report.name)
self.assertTrue(lint_task_report.failed)

def test_invalid_use_of_parent(self):
linter = general_purpose_linter.GeneralPurposeLinter(
[INVALID_PARENT_FILEPATH], FILE_CACHE)
lint_task_report = linter.check_bad_patterns()
self.assert_same_list_elements([
'Line 25: Please do not access parent properties using '
'$parent. Use the scope objectfor this purpose.'
], lint_task_report.trimmed_messages)
self.assertEqual('Bad pattern', lint_task_report.name)
self.assertTrue(lint_task_report.failed)

def test_invalid_use_of_relative_import(self):
linter = general_purpose_linter.GeneralPurposeLinter(
[INVALID_RELATIVE_IMPORT_FILEPATH], FILE_CACHE)
Expand All @@ -190,16 +176,6 @@ def test_invalid_use_of_inner_html(self):
self.assertEqual('Bad pattern', lint_task_report.name)
self.assertTrue(lint_task_report.failed)

def test_invalid_use_of_broadcast(self):
linter = general_purpose_linter.GeneralPurposeLinter(
[INVALID_BROADCAST_USE_FILEPATH], FILE_CACHE)
lint_task_report = linter.check_bad_patterns()
self.assert_same_list_elements([
'Line 26: Please do not use $broadcast/$on for propagating events. '
'Use @Input/@Output instead.'], lint_task_report.trimmed_messages)
self.assertEqual('Bad pattern', lint_task_report.name)
self.assertTrue(lint_task_report.failed)

def test_invalid_lodash_general_import(self):
linter = general_purpose_linter.GeneralPurposeLinter(
[INVALID_LODASH_GENERAL_IMPORT_FILEPATH], FILE_CACHE)
Expand Down
30 changes: 0 additions & 30 deletions scripts/linters/test_files/invalid_broadcast_use.ts

This file was deleted.

26 changes: 0 additions & 26 deletions scripts/linters/test_files/invalid_parent.ts

This file was deleted.

0 comments on commit 90d0152

Please sign in to comment.