Skip to content

Commit

Permalink
Guard for-in loops and enable guard-for-in lint rule. (Leaflet#8879)
Browse files Browse the repository at this point in the history
  • Loading branch information
alope107 authored Apr 15, 2023
1 parent 2f0a4fe commit 423e4ab
Show file tree
Hide file tree
Showing 19 changed files with 133 additions and 67 deletions.
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ module.exports = {
'spaced-comment': 'error',
'strict': 'off',
'wrap-iife': 'off',
'guard-for-in': 'error',
// TODO: Re-enable the rules below and fix the linting issues.
'no-invalid-this': 'off',
'prefer-object-has-own': 'error',
Expand Down
4 changes: 2 additions & 2 deletions spec/suites/dom/DomEvent.DoubleTapSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ describe('DomEvent.DoubleTapSpec.js', () => {
detail: 2,
target: container
});
for (const prop in expectedProps) {
expect(event[prop]).to.be(expectedProps[prop]);
for (const [prop, expectedValue] of Object.entries(expectedProps)) {
expect(event[prop]).to.be(expectedValue);
}
expect(event.isTrusted).not.to.be.ok();
});
Expand Down
8 changes: 5 additions & 3 deletions spec/suites/dom/DomEvent.PointerSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,11 @@ describe('DomEvent.Pointer', () => {
}
let res;
for (const prop in props) {
res = true;
if (props[prop] !== evt[prop]) {
return false;
if (Object.hasOwn(props, prop)) {
res = true;
if (props[prop] !== evt[prop]) {
return false;
}
}
}
return res;
Expand Down
4 changes: 2 additions & 2 deletions spec/suites/layer/vector/PolygonSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -355,8 +355,8 @@ describe('Polygon', () => {
polygon.addTo(map);
polygon.setStyle(style);

for (const prop in style) {
expect(polygon.options[prop]).to.be(style[prop]);
for (const [prop, expectedValue] of Object.entries(style)) {
expect(polygon.options[prop]).to.be(expectedValue);
}
});
});
Expand Down
8 changes: 4 additions & 4 deletions spec/suites/layer/vector/PolylineSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -247,8 +247,8 @@ describe('Polyline', () => {
polyline.addTo(map);
polyline.setStyle(style);

for (const prop in style) {
expect(polyline.options[prop]).to.be(style[prop]);
for (const [prop, expectedValue] of Object.entries(style)) {
expect(polyline.options[prop]).to.be(expectedValue);
}
});
});
Expand All @@ -263,8 +263,8 @@ describe('Polyline', () => {
polyline.addTo(map);
polyline.setStyle(style);

for (const prop in style) {
expect(polyline.options[prop]).to.be(style[prop]);
for (const [prop, expectedValue] of Object.entries(style)) {
expect(polyline.options[prop]).to.be(expectedValue);
}
});
});
Expand Down
4 changes: 2 additions & 2 deletions spec/suites/map/handler/Map.TapHoldSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,8 @@ describe('Map.TapHoldSpec.js', () => {
cancelable: true,
target: container
}, posStart);
for (const prop in expectedProps) {
expect(originalEvent[prop]).to.be(expectedProps[prop]);
for (const [prop, expectedValue] of Object.entries(expectedProps)) {
expect(originalEvent[prop]).to.be(expectedValue);
}
});
});
8 changes: 6 additions & 2 deletions src/control/Control.Layers.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,15 @@ export const Layers = Control.extend({
this._handlingClick = false;

for (const i in baseLayers) {
this._addLayer(baseLayers[i], i);
if (Object.hasOwn(baseLayers, i)) {
this._addLayer(baseLayers[i], i);
}
}

for (const i in overlays) {
this._addLayer(overlays[i], i, true);
if (Object.hasOwn(overlays, i)) {
this._addLayer(overlays[i], i, true);
}
}
},

Expand Down
4 changes: 3 additions & 1 deletion src/control/Control.js
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,9 @@ Map.include({

_clearControlPos() {
for (const i in this._controlCorners) {
this._controlCorners[i].remove();
if (Object.hasOwn(this._controlCorners, i)) {
this._controlCorners[i].remove();
}
}
this._controlContainer.remove();
delete this._controlCorners;
Expand Down
30 changes: 19 additions & 11 deletions src/core/Events.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,11 @@ export const Events = {
// types can be a map of types/handlers
if (typeof types === 'object') {
for (const type in types) {
// we don't process space-separated events here for performance;
// it's a hot path since Layer uses the on(obj) syntax
this._on(type, types[type], fn);
if (Object.hasOwn(types, type)) {
// we don't process space-separated events here for performance;
// it's a hot path since Layer uses the on(obj) syntax
this._on(type, types[type], fn);
}
}

} else {
Expand Down Expand Up @@ -75,7 +77,9 @@ export const Events = {

} else if (typeof types === 'object') {
for (const type in types) {
this._off(type, types[type], fn);
if (Object.hasOwn(types, type)) {
this._off(type, types[type], fn);
}
}

} else {
Expand Down Expand Up @@ -272,9 +276,11 @@ export const Events = {
// types can be a map of types/handlers
if (typeof types === 'object') {
for (const type in types) {
// we don't process space-separated events here for performance;
// it's a hot path since Layer uses the on(obj) syntax
this._on(type, types[type], fn, true);
if (Object.hasOwn(types, type)) {
// we don't process space-separated events here for performance;
// it's a hot path since Layer uses the on(obj) syntax
this._on(type, types[type], fn, true);
}
}

} else {
Expand Down Expand Up @@ -308,10 +314,12 @@ export const Events = {

_propagateEvent(e) {
for (const id in this._eventParents) {
this._eventParents[id].fire(e.type, Util.extend({
layer: e.target,
propagatedFrom: e.target
}, e), true);
if (Object.hasOwn(this._eventParents, id)) {
this._eventParents[id].fire(e.type, Util.extend({
layer: e.target,
propagatedFrom: e.target
}, e), true);
}
}
}
};
Expand Down
17 changes: 12 additions & 5 deletions src/core/Util.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,16 @@
*/

// @function extend(dest: Object, src?: Object): Object
// Merges the properties of the `src` object (or multiple objects) into `dest` object and returns the latter. Has an `L.extend` shortcut.
// Merges the properties (including properties inherited through the prototype chain)
// of the `src` object (or multiple objects) into `dest` object and returns the latter.
// Has an `L.extend` shortcut.
export function extend(dest, ...args) {
let i, j, len, src;
let j, len, src;

for (j = 0, len = args.length; j < len; j++) {
src = args[j];
for (i in src) {
// eslint-disable-next-line guard-for-in
for (const i in src) {
dest[i] = src[i];
}
}
Expand Down Expand Up @@ -104,7 +107,9 @@ export function setOptions(obj, options) {
obj.options = obj.options ? Object.create(obj.options) : {};
}
for (const i in options) {
obj.options[i] = options[i];
if (Object.hasOwn(options, i)) {
obj.options[i] = options[i];
}
}
return obj.options;
}
Expand All @@ -117,7 +122,9 @@ export function setOptions(obj, options) {
export function getParamString(obj, existingUrl, uppercase) {
const params = [];
for (const i in obj) {
params.push(`${encodeURIComponent(uppercase ? i.toUpperCase() : i)}=${encodeURIComponent(obj[i])}`);
if (Object.hasOwn(obj, i)) {
params.push(`${encodeURIComponent(uppercase ? i.toUpperCase() : i)}=${encodeURIComponent(obj[i])}`);
}
}
return ((!existingUrl || !existingUrl.includes('?')) ? '?' : '&') + params.join('&');
}
Expand Down
4 changes: 3 additions & 1 deletion src/dom/DomEvent.Pointer.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,9 @@ function _handlePointer(handler, e) {

e.touches = [];
for (const i in _pointers) {
e.touches.push(_pointers[i]);
if (Object.hasOwn(_pointers, i)) {
e.touches.push(_pointers[i]);
}
}
e.changedTouches = [e];

Expand Down
16 changes: 9 additions & 7 deletions src/dom/DomEvent.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ import {getScale} from './DomUtil.js';
export function on(obj, types, fn, context) {

if (types && typeof types === 'object') {
for (const type in types) {
addOne(obj, type, types[type], fn);
for (const [type, listener] of Object.entries(types)) {
addOne(obj, type, listener, fn);
}
} else {
types = Util.splitWords(types);
Expand Down Expand Up @@ -63,8 +63,8 @@ export function off(obj, types, fn, context) {
delete obj[eventsKey];

} else if (types && typeof types === 'object') {
for (const type in types) {
removeOne(obj, type, types[type], fn);
for (const [type, listener] of Object.entries(types)) {
removeOne(obj, type, listener, fn);
}

} else {
Expand All @@ -84,9 +84,11 @@ export function off(obj, types, fn, context) {

function batchRemove(obj, filterFn) {
for (const id in obj[eventsKey]) {
const type = id.split(/\d/)[0];
if (!filterFn || filterFn(type)) {
removeOne(obj, type, null, null, id);
if (Object.hasOwn(obj[eventsKey], id)) {
const type = id.split(/\d/)[0];
if (!filterFn || filterFn(type)) {
removeOne(obj, type, null, null, id);
}
}
}
}
Expand Down
6 changes: 4 additions & 2 deletions src/layer/FeatureGroup.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,10 @@ export const FeatureGroup = LayerGroup.extend({
const bounds = new LatLngBounds();

for (const id in this._layers) {
const layer = this._layers[id];
bounds.extend(layer.getBounds ? layer.getBounds() : layer.getLatLng());
if (Object.hasOwn(this._layers, id)) {
const layer = this._layers[id];
bounds.extend(layer.getBounds ? layer.getBounds() : layer.getLatLng());
}
}
return bounds;
}
Expand Down
12 changes: 8 additions & 4 deletions src/layer/Layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,9 @@ Map.include({
*/
eachLayer(method, context) {
for (const i in this._layers) {
method.call(context, this._layers[i]);
if (Object.hasOwn(this._layers, i)) {
method.call(context, this._layers[i]);
}
}
return this;
},
Expand Down Expand Up @@ -248,10 +250,12 @@ Map.include({
const oldZoomSpan = this._getZoomSpan();

for (const i in this._zoomBoundLayers) {
const options = this._zoomBoundLayers[i].options;
if (Object.hasOwn(this._zoomBoundLayers, i)) {
const options = this._zoomBoundLayers[i].options;

minZoom = options.minZoom === undefined ? minZoom : Math.min(minZoom, options.minZoom);
maxZoom = options.maxZoom === undefined ? maxZoom : Math.max(maxZoom, options.maxZoom);
minZoom = options.minZoom === undefined ? minZoom : Math.min(minZoom, options.minZoom);
maxZoom = options.maxZoom === undefined ? maxZoom : Math.max(maxZoom, options.maxZoom);
}
}

this._layersMaxZoom = maxZoom === -Infinity ? undefined : maxZoom;
Expand Down
12 changes: 8 additions & 4 deletions src/layer/LayerGroup.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,12 @@ export const LayerGroup = Layer.extend({
let i, layer;

for (i in this._layers) {
layer = this._layers[i];
if (Object.hasOwn(this._layers, i)) {
layer = this._layers[i];

if (layer[methodName]) {
layer[methodName].apply(layer, args);
if (layer[methodName]) {
layer[methodName].apply(layer, args);
}
}
}

Expand All @@ -118,7 +120,9 @@ export const LayerGroup = Layer.extend({
// ```
eachLayer(method, context) {
for (const i in this._layers) {
method.call(context, this._layers[i]);
if (Object.hasOwn(this._layers, i)) {
method.call(context, this._layers[i]);
}
}
return this;
},
Expand Down
Loading

0 comments on commit 423e4ab

Please sign in to comment.