From 5654f1b6b094abbdcbd554284ba2a259f61d6abf Mon Sep 17 00:00:00 2001 From: Angelo Silvestre Date: Fri, 21 Feb 2025 03:43:38 -0300 Subject: [PATCH] [Spellcheck] Add tests for ignore rules (Resolves #2573) (#2574) --- .github/workflows/pr_validation.yaml | 23 +- .../spelling_and_grammar_plugin.dart | 298 ++++++++++-------- super_editor_spellcheck/pubspec.yaml | 3 +- .../test/spellcheck_ignore_rules_test.dart | 221 +++++++++++++ 4 files changed, 413 insertions(+), 132 deletions(-) create mode 100644 super_editor_spellcheck/test/spellcheck_ignore_rules_test.dart diff --git a/.github/workflows/pr_validation.yaml b/.github/workflows/pr_validation.yaml index d5948a41c..d57295b72 100644 --- a/.github/workflows/pr_validation.yaml +++ b/.github/workflows/pr_validation.yaml @@ -144,6 +144,26 @@ jobs: # Run all tests - run: flutter test + test_super_editor_spellcheck: + runs-on: ubuntu-latest + defaults: + run: + working-directory: ./super_editor_spellcheck + steps: + # Checkout the PR branch + - uses: actions/checkout@v3 + + # Setup Flutter environment + - uses: subosito/flutter-action@v2 + with: + channel: "master" + + # Download all the packages that the app uses + - run: flutter pub get + + # Run all tests + - run: flutter test + analyze_super_keyboard: runs-on: ubuntu-latest defaults: @@ -184,7 +204,6 @@ jobs: # Run all tests - run: flutter test - analyze_super_text_layout: runs-on: ubuntu-latest defaults: @@ -288,4 +307,4 @@ jobs: - run: dart pub get # Run all tests - - run: dart test \ No newline at end of file + - run: dart test diff --git a/super_editor_spellcheck/lib/src/super_editor/spelling_and_grammar_plugin.dart b/super_editor_spellcheck/lib/src/super_editor/spelling_and_grammar_plugin.dart index 51417226c..6536520a5 100644 --- a/super_editor_spellcheck/lib/src/super_editor/spelling_and_grammar_plugin.dart +++ b/super_editor_spellcheck/lib/src/super_editor/spelling_and_grammar_plugin.dart @@ -37,6 +37,12 @@ class SpellingAndGrammarPlugin extends SuperEditorPlugin { /// - [ignoreRules]: a list of rules that determine ranges that should be ignored from spellchecking. /// It can be used, for example, to ignore links or text with specific attributions. See [SpellingIgnoreRules] /// for a list of built-in rules. + /// - [spellCheckService]: a spell check service to use for spell checking. If this is provided, + /// the plugin will use this service instead of the default spell check service. The default spell checker + /// supports macOS, Android, and iOS. + /// - [grammarCheckService]: a grammar check service to use for grammar checking. If this is provided, + /// the plugin will use this service instead of the default grammar check service. The default grammar checker + /// supports macOS only. SpellingAndGrammarPlugin({ bool isSpellingCheckEnabled = true, UnderlineStyle spellingErrorUnderlineStyle = defaultSpellingErrorUnderlineStyle, @@ -47,6 +53,8 @@ class SpellingAndGrammarPlugin extends SuperEditorPlugin { SuperEditorAndroidControlsController? androidControlsController, SuperEditorIosControlsController? iosControlsController, List ignoreRules = const [], + SpellCheckService? spellCheckService, + GrammarCheckService? grammarCheckService, }) : _isSpellCheckEnabled = isSpellingCheckEnabled, _isGrammarCheckEnabled = isGrammarCheckEnabled { assert(defaultTargetPlatform != TargetPlatform.android || androidControlsController != null, @@ -55,6 +63,19 @@ class SpellingAndGrammarPlugin extends SuperEditorPlugin { assert(defaultTargetPlatform != TargetPlatform.iOS || iosControlsController != null, 'The iosControlsController must be provided when running on iOS.'); + _spellCheckService = spellCheckService ?? + switch (defaultTargetPlatform) { + TargetPlatform.macOS => MacSpellCheckService(), + TargetPlatform.android || TargetPlatform.iOS => DefaultSpellCheckService(), + _ => null, + }; + + _grammarCheckService = grammarCheckService ?? + switch (defaultTargetPlatform) { + TargetPlatform.macOS => MacGrammarCheckService(), + _ => null, + }; + documentOverlayBuilders = [ SpellingErrorSuggestionOverlayBuilder( _spellingErrorSuggestions, @@ -88,6 +109,12 @@ class SpellingAndGrammarPlugin extends SuperEditorPlugin { }; } + /// A service that provides spell checking functionality. + late final SpellCheckService? _spellCheckService; + + /// A service that provides grammar checking functionality. + late final GrammarCheckService? _grammarCheckService; + final _spellingErrorSuggestions = SpellingErrorSuggestions(); late final SpellingAndGrammarStyler _styler; @@ -147,7 +174,8 @@ class SpellingAndGrammarPlugin extends SuperEditorPlugin { editor.context.put(spellingErrorSuggestionsKey, _spellingErrorSuggestions); _contentTapHandler?.editor = editor; - _reaction = SpellingAndGrammarReaction(_spellingErrorSuggestions, _styler, _ignoreRules); + _reaction = SpellingAndGrammarReaction( + _spellingErrorSuggestions, _styler, _ignoreRules, _spellCheckService!, _grammarCheckService); editor.reactionPipeline.add(_reaction); // Do initial spelling and grammar analysis, in case the document already @@ -254,7 +282,8 @@ extension SpellingAndGrammarEditorExtensions on Editor { /// An [EditReaction] that runs spelling and grammar checks on all [TextNode]s /// in a given [Document]. class SpellingAndGrammarReaction implements EditReaction { - SpellingAndGrammarReaction(this._suggestions, this._styler, this._ignoreRules); + SpellingAndGrammarReaction( + this._suggestions, this._styler, this._ignoreRules, this._spellCheckService, this._grammarCheckService); final SpellingErrorSuggestions _suggestions; @@ -262,6 +291,10 @@ class SpellingAndGrammarReaction implements EditReaction { final List _ignoreRules; + final SpellCheckService _spellCheckService; + + final GrammarCheckService? _grammarCheckService; + bool isSpellCheckEnabled = true; set spellingErrorUnderlineStyle(UnderlineStyle style) => _styler.spellingErrorUnderlineStyle = style; @@ -279,9 +312,6 @@ class SpellingAndGrammarReaction implements EditReaction { /// of receipt. final _asyncRequestIds = {}; - final _mobileSpellChecker = DefaultSpellCheckService(); - final _macSpellChecker = SuperEditorSpellCheckerPlugin().macSpellChecker; - /// Checks every [TextNode] in the given document for spelling and grammar /// errors and stores them for visual styling. void analyzeWholeDocument(EditContext editorContext) { @@ -367,17 +397,8 @@ class SpellingAndGrammarReaction implements EditReaction { } Future _findSpellingAndGrammarErrors(TextNode textNode) async { - if (defaultTargetPlatform == TargetPlatform.macOS) { - await _findSpellingAndGrammarErrorsOnMac(textNode); - } else if (defaultTargetPlatform == TargetPlatform.android || defaultTargetPlatform == TargetPlatform.iOS) { - await _findSpellingAndGrammarErrorsOnMobile(textNode); - } - } - - Future _findSpellingAndGrammarErrorsOnMac(TextNode textNode) async { - // TODO: Investigate whether we can parallelize spelling and grammar checks - // for a given node (and whether it's worth the complexity). final textErrors = {}; + final spellingSuggestions = {}; // Track this spelling and grammar request to make sure we don't process // the response out of order with other requests. @@ -387,129 +408,51 @@ class SpellingAndGrammarReaction implements EditReaction { final plainText = _filterIgnoredRanges(textNode); - int startingOffset = 0; - TextRange prevError = TextRange.empty; - final locale = PlatformDispatcher.instance.locale; - final language = _macSpellChecker.convertDartLocaleToMacLanguageCode(locale)!; - final spellingSuggestions = {}; if (isSpellCheckEnabled) { - do { - prevError = await _macSpellChecker.checkSpelling( - stringToCheck: plainText, - startingOffset: startingOffset, - language: language, - ); - - if (prevError.isValid) { - final word = plainText.substring(prevError.start, prevError.end); - - // Ask platform for spelling correction guesses. - final guesses = await _macSpellChecker.guesses(range: prevError, text: plainText); - + final suggestions = await _spellCheckService.fetchSpellCheckSuggestions( + PlatformDispatcher.instance.locale, + plainText, + ); + if (suggestions != null) { + for (final suggestion in suggestions) { + final misspelledWord = plainText.substring(suggestion.range.start, suggestion.range.end); + spellingSuggestions[suggestion.range] = SpellingError( + word: misspelledWord, + nodeId: textNode.id, + range: suggestion.range, + suggestions: suggestion.suggestions, + ); textErrors.add( TextError.spelling( nodeId: textNode.id, - range: prevError, - value: word, - suggestions: guesses, + range: suggestion.range, + value: misspelledWord, + suggestions: suggestion.suggestions, ), ); - - spellingSuggestions[prevError] = SpellingError( - word: word, - nodeId: textNode.id, - range: prevError, - suggestions: guesses, - ); - - startingOffset = prevError.end; - } - } while (prevError.isValid); - } - - if (isGrammarCheckEnabled) { - startingOffset = 0; - prevError = TextRange.empty; - do { - final result = await _macSpellChecker.checkGrammar( - stringToCheck: plainText, - startingOffset: startingOffset, - language: language, - ); - prevError = result.firstError ?? TextRange.empty; - - if (prevError.isValid) { - for (final grammarError in result.details) { - final errorRange = grammarError.range; - final text = plainText.substring(errorRange.start, errorRange.end); - textErrors.add( - TextError.grammar( - nodeId: textNode.id, - range: errorRange, - value: text, - ), - ); - } - - startingOffset = prevError.end; } - } while (prevError.isValid); - } - - if (requestId != _asyncRequestIds[textNode.id]) { - // Another request was started for this node while we were running our - // request. Fizzle. - return; - } - // Reset the request ID counter to zero so that we avoid increasing infinitely. - _asyncRequestIds[textNode.id] = 0; - - // Display underlines on spelling and grammar errors. - _styler - ..clearErrorsForNode(textNode.id) - ..addErrors(textNode.id, textErrors); - - // Update the shared repository of spelling suggestions so that the user can - // see suggestions and select them. - _suggestions.putSuggestions(textNode.id, spellingSuggestions); - } - - Future _findSpellingAndGrammarErrorsOnMobile(TextNode textNode) async { - final textErrors = {}; - final spellingSuggestions = {}; - - // Track this spelling and grammar request to make sure we don't process - // the response out of order with other requests. - _asyncRequestIds[textNode.id] ??= 0; - final requestId = _asyncRequestIds[textNode.id]! + 1; - _asyncRequestIds[textNode.id] = requestId; - - final plainText = _filterIgnoredRanges(textNode); - - final suggestions = await _mobileSpellChecker.fetchSpellCheckSuggestions( - PlatformDispatcher.instance.locale, - plainText, - ); - if (suggestions == null) { - return; + } } - for (final suggestion in suggestions) { - final misspelledWord = plainText.substring(suggestion.range.start, suggestion.range.end); - spellingSuggestions[suggestion.range] = SpellingError( - word: misspelledWord, - nodeId: textNode.id, - range: suggestion.range, - suggestions: suggestion.suggestions, - ); - textErrors.add( - TextError.spelling( - nodeId: textNode.id, - range: suggestion.range, - value: misspelledWord, - suggestions: suggestion.suggestions, - ), + if (isGrammarCheckEnabled && _grammarCheckService != null) { + final grammarErrors = await _grammarCheckService!.checkGrammar( + PlatformDispatcher.instance.locale, + plainText, ); + + if (grammarErrors != null) { + for (final grammarError in grammarErrors) { + final errorRange = grammarError.range; + final text = plainText.substring(errorRange.start, errorRange.end); + textErrors.add( + TextError.grammar( + nodeId: textNode.id, + range: errorRange, + value: text, + ), + ); + } + } } if (requestId != _asyncRequestIds[textNode.id]) { @@ -912,3 +855,102 @@ class SpellingIgnoreRules { }; } } + +/// A [SpellCheckService] that uses a macOS plugin to check spelling. +class MacSpellCheckService implements SpellCheckService { + final _macSpellChecker = SuperEditorSpellCheckerPlugin().macSpellChecker; + + @override + Future?> fetchSpellCheckSuggestions(Locale locale, String text) async { + // TODO: Investigate whether we can parallelize spelling and grammar checks + // for a given node (and whether it's worth the complexity). + + final suggestionSpans = []; + + int startingOffset = 0; + TextRange prevError = TextRange.empty; + final locale = PlatformDispatcher.instance.locale; + final language = _macSpellChecker.convertDartLocaleToMacLanguageCode(locale)!; + do { + prevError = await _macSpellChecker.checkSpelling( + stringToCheck: text, + startingOffset: startingOffset, + language: language, + ); + + if (prevError.isValid) { + final guesses = await _macSpellChecker.guesses(range: prevError, text: text); + suggestionSpans.add(SuggestionSpan(prevError, guesses)); + startingOffset = prevError.end; + } + } while (prevError.isValid); + + return suggestionSpans; + } +} + +/// A service that knows how to check grammar on a text. +abstract class GrammarCheckService { + /// Checks the given [text] for grammar errors with the given [locale]. + /// + /// Returns a list of [GrammarError]s where each item represents a sentence + /// that has a grammatical error, with details about the error. + /// + /// Returns an empty list if no grammar errors are found or if the [locale] + /// isn't supported by the grammar checker. + /// + /// Returns `null` if the check was unsucessful. + Future?> checkGrammar(Locale locale, String text); +} + +/// A [GrammarCheckService] that uses a macOS plugin to check grammar. +class MacGrammarCheckService implements GrammarCheckService { + final _macSpellChecker = SuperEditorSpellCheckerPlugin().macSpellChecker; + + @override + Future?> checkGrammar(Locale locale, String text) async { + final errors = []; + + final language = _macSpellChecker.convertDartLocaleToMacLanguageCode(locale)!; + + int startingOffset = 0; + TextRange prevError = TextRange.empty; + do { + final result = await _macSpellChecker.checkGrammar( + stringToCheck: text, + startingOffset: startingOffset, + language: language, + ); + prevError = result.firstError ?? TextRange.empty; + + if (prevError.isValid) { + errors.addAll( + result.details.map( + (detail) => GrammarError( + range: detail.range, + description: detail.userDescription, + ), + ), + ); + + startingOffset = prevError.end; + } + } while (prevError.isValid); + + return errors; + } +} + +/// A grammatical error found in a text at [range]. +class GrammarError { + GrammarError({ + required this.range, + required this.description, + }); + + /// The range of text that has a grammatical error. + final TextRange range; + + /// The description of the grammatical error. + final String description; +} diff --git a/super_editor_spellcheck/pubspec.yaml b/super_editor_spellcheck/pubspec.yaml index 3828a088b..3b3e60fc4 100644 --- a/super_editor_spellcheck/pubspec.yaml +++ b/super_editor_spellcheck/pubspec.yaml @@ -15,8 +15,6 @@ dependencies: overlord: ^0.0.3+5 plugin_platform_interface: ^2.0.2 -# collection: ^1.18.0 -# follow_the_leader: ^0.0.4+8 super_editor: ^0.3.0-dev.15 super_text_layout: ^0.1.12 @@ -31,6 +29,7 @@ dev_dependencies: sdk: flutter flutter_lints: ^3.0.0 pigeon: ^21.1.0 + flutter_test_runners: ^0.0.4 # For information on the generic Dart part of this file, see the # following page: https://dart.dev/tools/pub/pubspec diff --git a/super_editor_spellcheck/test/spellcheck_ignore_rules_test.dart b/super_editor_spellcheck/test/spellcheck_ignore_rules_test.dart new file mode 100644 index 000000000..c6e85d7d1 --- /dev/null +++ b/super_editor_spellcheck/test/spellcheck_ignore_rules_test.dart @@ -0,0 +1,221 @@ +import 'dart:collection'; + +import 'package:flutter/material.dart'; +import 'package:flutter/services.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:flutter_test_runners/flutter_test_runners.dart'; +import 'package:super_editor/super_editor.dart'; +import 'package:super_editor_spellcheck/super_editor_spellcheck.dart'; + +void main() { + group('SuperEditor spellcheck >', () { + group('ignore rules >', () { + testWidgetsOnArbitraryDesktop('ignores by pattern', (tester) async { + final spellCheckerService = _FakeSpellChecker(); + + await _pumpTestApp( + tester, + document: MutableDocument( + nodes: [ + ParagraphNode( + id: Editor.createNodeId(), + text: AttributedText( + 'An user @mention and @another one', + ), + ), + ], + ), + ignoreRules: [ + // Ignores user mentions, like "@mention". + SpellingIgnoreRules.byPattern(RegExp(r'@\w+')), + ], + spellCheckerService: spellCheckerService, + ); + + // Ensure the spell checker service was queried without the text + // that matches the pattern. + expect(spellCheckerService.queriedTexts, ['An user and one']); + }); + + testWidgetsOnArbitraryDesktop('ignores by attribution', (tester) async { + final spellCheckerService = _FakeSpellChecker(); + + await _pumpTestApp( + tester, + document: MutableDocument( + nodes: [ + ParagraphNode( + id: Editor.createNodeId(), + text: AttributedText( + 'A bold text and another bold text', + AttributedSpans( + attributions: [ + // First "bold" word. + const SpanMarker( + attribution: boldAttribution, + offset: 2, + markerType: SpanMarkerType.start, + ), + const SpanMarker( + attribution: boldAttribution, + offset: 5, + markerType: SpanMarkerType.end, + ), + // Second "bold" word. + const SpanMarker( + attribution: boldAttribution, + offset: 24, + markerType: SpanMarkerType.start, + ), + const SpanMarker( + attribution: boldAttribution, + offset: 27, + markerType: SpanMarkerType.end, + ), + ], + ), + ), + ), + ], + ), + ignoreRules: [ + SpellingIgnoreRules.byAttribution(boldAttribution), + ], + spellCheckerService: spellCheckerService, + ); + + // Ensure the spell checker service was queried without the text + // that contains the bold attribution. + expect(spellCheckerService.queriedTexts, ['A text and another text']); + }); + + testWidgetsOnArbitraryDesktop('ignores by attribution filter', (tester) async { + final spellCheckerService = _FakeSpellChecker(); + + await _pumpTestApp( + tester, + document: MutableDocument( + nodes: [ + ParagraphNode( + id: Editor.createNodeId(), + text: AttributedText( + 'A link and another link', + AttributedSpans( + attributions: [ + // First link. + const SpanMarker( + attribution: LinkAttribution('https://www.google.com'), + offset: 2, + markerType: SpanMarkerType.start, + ), + const SpanMarker( + attribution: LinkAttribution('https://www.google.com'), + offset: 5, + markerType: SpanMarkerType.end, + ), + // Second link. + const SpanMarker( + attribution: LinkAttribution('https://www.youtube.com'), + offset: 19, + markerType: SpanMarkerType.start, + ), + const SpanMarker( + attribution: LinkAttribution('https://www.youtube.com'), + offset: 22, + markerType: SpanMarkerType.end, + ), + ], + ), + ), + ), + ], + ), + ignoreRules: [ + SpellingIgnoreRules.byAttributionFilter((attribution) => attribution is LinkAttribution), + ], + spellCheckerService: spellCheckerService, + ); + + // Ensure the spell checker service was queried without the text + // that contains the link attribution. + expect(spellCheckerService.queriedTexts, ['A and another ']); + }); + + testWidgetsOnArbitraryDesktop('allows overlapping rules', (tester) async { + final spellCheckerService = _FakeSpellChecker(); + + await _pumpTestApp( + tester, + document: MutableDocument( + nodes: [ + ParagraphNode( + id: Editor.createNodeId(), + text: AttributedText( + 'The first text and the second text', + ), + ), + ], + ), + ignoreRules: [ + (TextNode node) { + // The first text and the second text + // ^^^^^^^^ + return const [TextRange(start: 10, end: 18)]; + }, + // The first text and the second text + // ^^^^^^^^^^^^^^ + (TextNode node) { + return const [TextRange(start: 15, end: 29)]; + } + ], + spellCheckerService: spellCheckerService, + ); + + // Ensure the spell checker service was queried without the text + // of the overlapping ranges. + expect(spellCheckerService.queriedTexts, ['The first text']); + }); + }); + }); +} + +Future _pumpTestApp( + WidgetTester tester, { + required MutableDocument document, + List ignoreRules = const [], + SpellCheckService? spellCheckerService, +}) async { + final editor = createDefaultDocumentEditor( + document: document, + composer: MutableDocumentComposer(), + ); + + final plugin = SpellingAndGrammarPlugin( + ignoreRules: ignoreRules, + spellCheckService: spellCheckerService, + ); + + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: SuperEditor( + editor: editor, + plugins: {plugin}, + ), + ), + ), + ); +} + +/// A [SpellCheckService] that records the texts that were queried and returns +/// an empty list of suggestions for each query. +class _FakeSpellChecker extends SpellCheckService { + List get queriedTexts => UnmodifiableListView(_queriedTexts); + final List _queriedTexts = []; + + @override + Future?> fetchSpellCheckSuggestions(Locale locale, String text) async { + _queriedTexts.add(text); + return const []; + } +}